Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: 'env' command moved to 'env dump' subcommand #1920

Merged
merged 3 commits into from
Jan 18, 2023
+37 −29
Merged

CLI: 'env' command moved to 'env dump' subcommand #1920

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d3ecc70
CLI: 'env' command moved to 'env dump' subcommand
pda Jan 18, 2023
3a1f8aa
Top-level `env` Usage text fixed.
pda Jan 18, 2023
7f71f7c
agent.Expect("env") -> agent.Expect("env", "dump")
DrJosh9000 Jan 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 16 additions & 14 deletions clicommand/env.go → clicommand/env_dump.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,29 +9,31 @@ import (
"github.com/urfave/cli"
)

const envDescription = `Usage:
buildkite-agent env [options]
var EnvDumpHelpDescription = `Usage:
DrJosh9000 marked this conversation as resolved.
Show resolved Hide resolved
buildkite-agent env dump [options]

Description:
Prints out the environment of the current process as a JSON object, easily
parsable by other programs. Used when executing hooks to discover changes
that hooks make to the environment.

Example:
$ buildkite-agent env

Prints the environment passed into the process
`
$ buildkite-agent env dump --format json-pretty`

var EnvCommand = cli.Command{
Name: "env",
Usage: "Prints out the environment of the current process as a JSON object",
Description: envDescription,
type EnvDumpConfig struct {
}

var EnvDumpCommand = cli.Command{
Name: "dump",
Usage: "Print the environment of the current process as a JSON object",
Description: EnvDumpHelpDescription,
Flags: []cli.Flag{
cli.BoolFlag{
Name: "pretty",
Usage: "Pretty print the JSON output",
EnvVar: "BUILDKITE_AGENT_ENV_PRETTY",
cli.StringFlag{
Name: "format",
Usage: "Output format; json or json-pretty",
EnvVar: "BUILDKITE_AGENT_ENV_DUMP_FORMAT",
Value: "json",
},
},
Action: func(c *cli.Context) error {
Expand All @@ -45,7 +47,7 @@ var EnvCommand = cli.Command{
}

enc := json.NewEncoder(c.App.Writer)
if c.Bool("pretty") {
if c.String("format") == "json-pretty" {
enc.SetIndent("", " ")
}

Expand Down
12 changes: 6 additions & 6 deletions hook/scriptwrapper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,26 @@ const (

batchScript = `@echo off
SETLOCAL ENABLEDELAYEDEXPANSION
buildkite-agent env > "{{.BeforeEnvFileName}}"
buildkite-agent env dump > "{{.BeforeEnvFileName}}"
CALL "{{.PathToHook}}"
SET BUILDKITE_HOOK_EXIT_STATUS=!ERRORLEVEL!
SET BUILDKITE_HOOK_WORKING_DIR=%CD%
buildkite-agent env > "{{.AfterEnvFileName}}"
buildkite-agent env dump > "{{.AfterEnvFileName}}"
EXIT %BUILDKITE_HOOK_EXIT_STATUS%`

powershellScript = `$ErrorActionPreference = "STOP"
buildkite-agent env | Set-Content "{{.BeforeEnvFileName}}"
buildkite-agent env dump | Set-Content "{{.BeforeEnvFileName}}"
{{.PathToHook}}
if ($LASTEXITCODE -eq $null) {$Env:BUILDKITE_HOOK_EXIT_STATUS = 0} else {$Env:BUILDKITE_HOOK_EXIT_STATUS = $LASTEXITCODE}
$Env:BUILDKITE_HOOK_WORKING_DIR = $PWD | Select-Object -ExpandProperty Path
buildkite-agent env | Set-Content "{{.AfterEnvFileName}}"
buildkite-agent env dump | Set-Content "{{.AfterEnvFileName}}"
exit $Env:BUILDKITE_HOOK_EXIT_STATUS`

bashScript = `buildkite-agent env > "{{.BeforeEnvFileName}}"
bashScript = `buildkite-agent env dump > "{{.BeforeEnvFileName}}"
. "{{.PathToHook}}"
export BUILDKITE_HOOK_EXIT_STATUS=$?
export BUILDKITE_HOOK_WORKING_DIR=$PWD
buildkite-agent env > "{{.AfterEnvFileName}}"
buildkite-agent env dump > "{{.AfterEnvFileName}}"
exit $BUILDKITE_HOOK_EXIT_STATUS`
)

Expand Down
12 changes: 6 additions & 6 deletions hook/scriptwrapper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,11 @@ func TestHookScriptsAreGeneratedCorrectlyOnWindowsBatch(t *testing.T) {
// See: https://pkg.go.dev/fmt > ctrl-f for "%%"
scriptTemplate := `@echo off
SETLOCAL ENABLEDELAYEDEXPANSION
buildkite-agent env > "%s"
buildkite-agent env dump > "%s"
CALL "%s"
SET BUILDKITE_HOOK_EXIT_STATUS=!ERRORLEVEL!
SET BUILDKITE_HOOK_WORKING_DIR=%%CD%%
buildkite-agent env > "%s"
buildkite-agent env dump > "%s"
EXIT %%BUILDKITE_HOOK_EXIT_STATUS%%`

assertScriptLike(t, scriptTemplate, hookFile.Name(), wrapper)
Expand All @@ -134,11 +134,11 @@ func TestHookScriptsAreGeneratedCorrectlyOnWindowsPowershell(t *testing.T) {
defer wrapper.Close()

scriptTemplate := `$ErrorActionPreference = "STOP"
buildkite-agent env | Set-Content "%s"
buildkite-agent env dump | Set-Content "%s"
%s
if ($LASTEXITCODE -eq $null) {$Env:BUILDKITE_HOOK_EXIT_STATUS = 0} else {$Env:BUILDKITE_HOOK_EXIT_STATUS = $LASTEXITCODE}
$Env:BUILDKITE_HOOK_WORKING_DIR = $PWD | Select-Object -ExpandProperty Path
buildkite-agent env | Set-Content "%s"
buildkite-agent env dump | Set-Content "%s"
exit $Env:BUILDKITE_HOOK_EXIT_STATUS`

assertScriptLike(t, scriptTemplate, hookFile.Name(), wrapper)
Expand All @@ -163,11 +163,11 @@ func TestHookScriptsAreGeneratedCorrectlyOnUnix(t *testing.T) {

defer wrapper.Close()

scriptTemplate := `buildkite-agent env > "%s"
scriptTemplate := `buildkite-agent env dump > "%s"
. "%s"
export BUILDKITE_HOOK_EXIT_STATUS=$?
export BUILDKITE_HOOK_WORKING_DIR=$PWD
buildkite-agent env > "%s"
buildkite-agent env dump > "%s"
exit $BUILDKITE_HOOK_EXIT_STATUS`

assertScriptLike(t, scriptTemplate, hookFile.Name(), wrapper)
Expand Down
8 changes: 7 additions & 1 deletion main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,13 @@ func main() {
clicommand.ArtifactShasumCommand,
},
},
{
Name: "env",
Usage: "Something something environment variables",
Subcommands: []cli.Command{
clicommand.EnvDumpCommand,
},
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
{
Name: "env",
Usage: "Something something environment variables",
Subcommands: []cli.Command{
clicommand.EnvDumpCommand,
},
},
{
Name: "env",
Usage: "Something something environment variables",
Subcommands: []cli.Command{
clicommand.EnvDumpCommand,
},
Action: clicommand.EnvDumpAction,
},

I think something like this (with appropriate changes in clicommand/env_dump.go) should work to allow buildkite-agent env and buildkite-agent env dump to be synonymous.

Not only does it preserve backward compatibility (up to the flag changes you made), but I actually think this is desirable behaviour in the long term.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but I actually think this is desirable behaviour in the long term

I wonder. I don't think dumping the process' current env is very useful outside the one weird use-case we have internally, which we hope to eliminate via control sockets. I can't imagine a user actually wanting to use buildkite-agent env to see their environment as JSON. I'd rather it show the usage and list of subcommands.

{
Name: "meta-data",
Usage: "Get/set data from Buildkite jobs",
Expand Down Expand Up @@ -114,7 +121,6 @@ func main() {
clicommand.StepUpdateCommand,
},
},
clicommand.EnvCommand,
clicommand.BootstrapCommand,
}

Expand Down