-
Notifications
You must be signed in to change notification settings - Fork 252
Add --envrc-dir flag to allow specifying location of direnv config #2629
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
415686e
6da0d57
df12f68
d005a8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,6 +6,7 @@ package boxcli | |||||
import ( | ||||||
"cmp" | ||||||
"fmt" | ||||||
"path/filepath" | ||||||
"regexp" | ||||||
|
||||||
"github.com/pkg/errors" | ||||||
|
@@ -24,6 +25,7 @@ type generateCmdFlags struct { | |||||
force bool | ||||||
printEnvrcContent bool | ||||||
rootUser bool | ||||||
envrcDir string // only used by generate direnv command | ||||||
} | ||||||
|
||||||
type generateDockerfileCmdFlags struct { | ||||||
|
@@ -147,10 +149,22 @@ func direnvCmd() *cobra.Command { | |||||
command.Flags().BoolVarP( | ||||||
&flags.force, "force", "f", false, "force overwrite existing files") | ||||||
command.Flags().BoolVarP( | ||||||
&flags.printEnvrcContent, "print-envrc", "p", false, "output contents of devbox configuration to use in .envrc") | ||||||
&flags.printEnvrcContent, "print-envrc", "p", false, | ||||||
"output contents of devbox configuration to use in .envrc") | ||||||
// this command marks a flag as hidden. Error handling for it is not necessary. | ||||||
_ = command.Flags().MarkHidden("print-envrc") | ||||||
|
||||||
// --envrc-dir allows users to specify a directory where the .envrc file should be generated | ||||||
// separately from the devbox config directory. Without this flag, the .envrc file | ||||||
// will be generated in the same directory as the devbox config file (i.e., either the current | ||||||
// directory or the directory specified by --config). This is useful for users who want to keep | ||||||
// their .envrc and devbox config files in different locations. | ||||||
command.Flags().StringVar( | ||||||
&flags.envrcDir, "envrc-dir", "", | ||||||
"path to directory where the .envrc file should be generated. "+ | ||||||
"If not specified, the .envrc file will be generated in "+ | ||||||
"the current working directory.") | ||||||
|
||||||
flags.config.register(command) | ||||||
return command | ||||||
} | ||||||
|
@@ -266,13 +280,32 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error { | |||||
} | ||||||
|
||||||
func runGenerateDirenvCmd(cmd *cobra.Command, flags *generateCmdFlags) error { | ||||||
// --print-envrc is used within the .envrc file and therefore doesn't make sense to also | ||||||
// use it with --envrc-dir, which specifies a directory where the .envrc file should be generated. | ||||||
if flags.printEnvrcContent && flags.envrcDir != "" { | ||||||
return usererr.New( | ||||||
"Cannot use --print-envrc with --envrc-dir. " + | ||||||
"Use --envrc-dir to specify the directory where the .envrc file should be generated.") | ||||||
pstephan-geico-external marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
} | ||||||
|
||||||
// Determine the directories for .envrc and config | ||||||
configDir, envrcDir, err := determineDirenvDirs(flags.config.path, flags.envrcDir) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think |
||||||
if err != nil { | ||||||
return errors.WithStack(err) | ||||||
} | ||||||
|
||||||
generateOpts := devopt.EnvrcOpts{ | ||||||
EnvrcDir: envrcDir, | ||||||
ConfigDir: configDir, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to supply There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is also a relative path (but named |
||||||
EnvFlags: devopt.EnvFlags(flags.envFlag), | ||||||
} | ||||||
|
||||||
if flags.printEnvrcContent { | ||||||
return devbox.PrintEnvrcContent( | ||||||
cmd.OutOrStdout(), devopt.EnvFlags(flags.envFlag)) | ||||||
return devbox.PrintEnvrcContent(cmd.OutOrStdout(), generateOpts) | ||||||
} | ||||||
|
||||||
box, err := devbox.Open(&devopt.Opts{ | ||||||
Dir: flags.config.path, | ||||||
Dir: filepath.Join(envrcDir, configDir), | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. keep this as |
||||||
Environment: flags.config.environment, | ||||||
Stderr: cmd.ErrOrStderr(), | ||||||
}) | ||||||
|
@@ -281,5 +314,37 @@ func runGenerateDirenvCmd(cmd *cobra.Command, flags *generateCmdFlags) error { | |||||
} | ||||||
|
||||||
return box.GenerateEnvrcFile( | ||||||
cmd.Context(), flags.force, devopt.EnvFlags(flags.envFlag)) | ||||||
cmd.Context(), flags.force, generateOpts) | ||||||
} | ||||||
|
||||||
// Returns cononical paths for configDir and envrcDir. Both locations are relative to the current | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix the typo 'cononical' to 'canonical' in the comment for
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||
// working directory when provided to this function. However, since the config file will ultimately | ||||||
// be relative to the .envrc file, we need to determine the relative path from envrcDir to configDir. | ||||||
func determineDirenvDirs(configDir, envrcDir string) (string, string, error) { | ||||||
// If envrcDir is not specified, we will use the configDir as the location for .envrc. This is | ||||||
// for backward compatibility (prior to the --envrc-dir flag being introduced). | ||||||
if envrcDir == "" { | ||||||
return "", configDir, nil | ||||||
} | ||||||
|
||||||
// If no configDir is specified, it will be assumed to be in the same directory as the .envrc file | ||||||
// which means we can just return an empty configDir. | ||||||
if configDir == "" { | ||||||
return "", envrcDir, nil | ||||||
} | ||||||
Comment on lines
+330
to
+334
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I agree with the semantics of this. --envrc-dir should dictate where the In practice that means that using |
||||||
|
||||||
relativeConfigDir, err := filepath.Rel(envrcDir, configDir) | ||||||
if err != nil { | ||||||
return "", "", errors.Wrapf(err, "failed to determine relative path from %s to %s", envrcDir, configDir) | ||||||
} | ||||||
|
||||||
// If the relative path is ".", it means configDir is the same as envrcDir. Leaving it as "." | ||||||
// will result in the .envrc containing "--config .", which is fine, but unnecessary and also | ||||||
// a change from the previous behavior. So we will return an empty string for relativeConfigDir | ||||||
// which will result in the .envrc file not containing the "--config" flag at all. | ||||||
if relativeConfigDir == "." { | ||||||
relativeConfigDir = "" | ||||||
} | ||||||
Comment on lines
+336
to
+347
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're computing the relative path, and then joining again at the call site. Instead of precomputing the relative path, I think it would be simpler to do that when the .envrc is generated. |
||||||
|
||||||
return relativeConfigDir, envrcDir, nil | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -140,32 +140,33 @@ func (g *Options) CreateDevcontainer(ctx context.Context) error { | |
return err | ||
} | ||
|
||
func CreateEnvrc(ctx context.Context, path string, envFlags devopt.EnvFlags) error { | ||
func CreateEnvrc(ctx context.Context, opts devopt.EnvrcOpts) error { | ||
defer trace.StartRegion(ctx, "createEnvrc").End() | ||
|
||
// create .envrc file | ||
file, err := os.Create(filepath.Join(path, ".envrc")) | ||
file, err := os.Create(filepath.Join(opts.EnvrcDir, ".envrc")) | ||
if err != nil { | ||
return err | ||
} | ||
defer file.Close() | ||
|
||
flags := []string{} | ||
|
||
if len(envFlags.EnvMap) > 0 { | ||
for k, v := range envFlags.EnvMap { | ||
if len(opts.EnvMap) > 0 { | ||
for k, v := range opts.EnvMap { | ||
flags = append(flags, fmt.Sprintf("--env %s=%s", k, v)) | ||
} | ||
} | ||
if envFlags.EnvFile != "" { | ||
flags = append(flags, fmt.Sprintf("--env-file %s", envFlags.EnvFile)) | ||
if opts.EnvFile != "" { | ||
flags = append(flags, fmt.Sprintf("--env-file %s", opts.EnvFile)) | ||
} | ||
|
||
t := template.Must(template.ParseFS(tmplFS, "tmpl/envrc.tmpl")) | ||
|
||
// write content into file | ||
return t.Execute(file, map[string]string{ | ||
"Flags": strings.Join(flags, " "), | ||
"Dir": opts.ConfigDir, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Name this Compute relative path here instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make empty if |
||
}) | ||
} | ||
|
||
|
@@ -219,7 +220,7 @@ func (g *Options) getDevcontainerContent() *devcontainerObject { | |
return devcontainerContent | ||
} | ||
|
||
func EnvrcContent(w io.Writer, envFlags devopt.EnvFlags) error { | ||
func EnvrcContent(w io.Writer, envFlags devopt.EnvrcOpts) error { | ||
tmplName := "envrcContent.tmpl" | ||
t := template.Must(template.ParseFS(tmplFS, "tmpl/"+tmplName)) | ||
envFlag := "" | ||
|
@@ -231,5 +232,6 @@ func EnvrcContent(w io.Writer, envFlags devopt.EnvFlags) error { | |
return t.Execute(w, map[string]string{ | ||
"EnvFlag": envFlag, | ||
"EnvFile": envFlags.EnvFile, | ||
"Dir": envFlags.EnvrcDir, | ||
}) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,7 @@ | ||
{{define "DirPrefix"}}{{ if .Dir }}{{ .Dir }}/{{ end }}{{end}} | ||
use_devbox() { | ||
watch_file devbox.json devbox.lock | ||
eval "$(devbox shellenv --init-hook --install --no-refresh-alias{{ if .EnvFlag }} {{ .EnvFlag }}{{ end }})" | ||
watch_file {{template "DirPrefix" .}}devbox.json {{template "DirPrefix" .}}devbox.lock | ||
eval "$(devbox shellenv --init-hook --install --no-refresh-alias {{ if .EnvFlag }}{{ .EnvFlag }}{{ end }} {{- if .Dir }}--config {{ .Dir -}}{{ end }})" | ||
Comment on lines
+1
to
+4
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Templates are pretty hard to read. Ideally this is computed in Go code and just passed in. e.g.
You could do similar with |
||
} | ||
use devbox | ||
{{ if .EnvFile }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
# Testscript to validate generating a direnv .envrc in a specified location (./dir) that also | ||
# references a devbox config in another dir (./cfg) that is a sibling to the first. | ||
|
||
mkdir cfg | ||
exec devbox init cfg | ||
exists cfg/devbox.json | ||
|
||
mkdir dir | ||
exec devbox generate direnv --envrc-dir dir --config cfg | ||
grep 'eval "\$\(devbox generate direnv --print-envrc --config ../cfg\)"' dir/.envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Testscript to validate generating a direnv .envrc in a specified location (./dir) that also | ||
# references a devbox config in another dir (./cfg) that is a subdir of the first. | ||
|
||
mkdir dir/cfg | ||
exec devbox init dir/cfg | ||
exists dir/cfg/devbox.json | ||
|
||
exec devbox generate direnv --envrc-dir dir --config dir/cfg | ||
grep 'eval "\$\(devbox generate direnv --print-envrc --config cfg\)"' dir/.envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Testscript to validate generating a direnv .envrc in the current location that | ||
# references a devbox config in a subdir (./cfg). | ||
|
||
mkdir cfg | ||
exec devbox init cfg | ||
exists cfg/devbox.json | ||
|
||
exec devbox generate direnv --envrc-dir . --config cfg | ||
grep 'eval "\$\(devbox generate direnv --print-envrc --config cfg\)"' ./.envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
# Testscript to validate generating a direnv .envrc in a specified location (./cfg) that also | ||
# references a devbox config in the same location (no --config needed) | ||
|
||
mkdir cfg | ||
exec devbox init cfg | ||
exists cfg/devbox.json | ||
|
||
exec devbox generate direnv --envrc-dir cfg | ||
grep 'eval "\$\(devbox generate direnv --print-envrc\)"' cfg/.envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# Testscript to validate generating the contents of the .envrc file. In this case the | ||
# config location is ignored because there is no --envrc-dir param. | ||
|
||
exec devbox init | ||
exec devbox generate direnv --print-envrc --config config-dir | ||
cp stdout results.txt | ||
grep 'watch_file config-dir/devbox.json config-dir/devbox.lock' results.txt | ||
grep 'eval "\$\(devbox shellenv --init-hook --install --no-refresh-alias --config config-dir\)"' results.txt | ||
! exists .envrc | ||
! exists config-dir | ||
! exists config-dir/.envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
# Testscript to validate that the --print-envrc and --envrc-dir params are not allowed | ||
# to be used at the same time. | ||
|
||
exec devbox init | ||
! exec devbox generate direnv --print-envrc --envrc-dir dir | ||
stderr 'Cannot use --print-envrc with --envrc-dir' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# Testscript to validate the output of --print-envrc | ||
|
||
exec devbox init | ||
exec devbox generate direnv --print-envrc | ||
cp stdout results.txt | ||
grep 'watch_file devbox.json devbox.lock' results.txt | ||
grep 'eval "\$\(devbox shellenv --init-hook --install --no-refresh-alias \)"' results.txt | ||
! exists .envrc |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,7 @@ | ||
# Testscript to validate generating the contents of the .envrc file. | ||
|
||
exec devbox init | ||
exec devbox generate direnv | ||
exists .envrc | ||
|
||
exists devbox.json | ||
grep 'eval "\$\(devbox generate direnv --print-envrc\)"' .envrc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be:
Since
--config
will determine where the .envrc file is created, which may not match the working directory.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also clarify that if this flag is used, then we use relative path for
--config