Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: Find config file with $CHEZMOI_CONFIG #2773

Closed
wants to merge 4 commits into from

Conversation

halostatue
Copy link
Collaborator

Closes #2771

This provides an alternative to pure command-line flag configuration when working with multiple source trees.

@halostatue
Copy link
Collaborator Author

This needs further work, especially on documentation and tests. I have yet had no time to consider what testing this might look like.

@twpayne
Copy link
Owner

twpayne commented Feb 14, 2023

I think that the code in this PR will cause the values read from the environment variables to be overwritten with values from the configuration file. I think the hook needs to happen at the end of ./pkg/cmd.Config.readConfig.

@halostatue
Copy link
Collaborator Author

I think that the code in this PR will cause the values read from the environment variables to be overwritten with values from the configuration file. I think the hook needs to happen at the end of ./pkg/cmd.Config.readConfig.

It’s possible — even probable. I think that will require some additional thinking and tracking to identify where the value came from. On the other hand, if most of these values could be found in a configuration file, then the only thing that might be necessary is $CHEZMOI_CONFIG — and the recommendation is that the config file has the necessary configuration to override everything else. This was mostly pushed for the moment to start a code discussion. I’ll look at refining it over the next day or two.

@halostatue
Copy link
Collaborator Author

I’m reworking this so that some of the variable names better match the corresponding command-line flags and configuration (instead of CHEZMOI_CACHE_PATH, it will be CHEZMOI_CACHE_DIR, etc.).

Some inconsistencies as I’m looking through this:

  • --persistent-state does not have a corresponding value in the config file.
    • Should persistentState be added to the config file? If not, should we remove $CHEZMOI_PERSISTENT_STATE?
  • scriptTempDir does not have a corresponding command-line flag.
    • Should one be added? Should we have $CHEZMOI_SCRIPT_TEMP_DIR?

Since most of the flags are available in the configuration file, I think we should mark the following flags as deprecated and remove them in Chezmoi v3:

  • --cache (use cacheDir instead)
  • --destination (use destDir instead)
  • --persistent-state (add persistentState to configuration file)
  • --source (use sourceDir instead)
  • --working-tree (use workingTree instead)

With the deprecation of these flags, we can change this to only support $CHEZMOI_CONFIG, which could be used instead of --config.

What do you think @twpayne?

@halostatue halostatue force-pushed the environment-overrides branch 3 times, most recently from 90e8d74 to 10600cc Compare March 8, 2023 17:28
Resolves twpayne#2771

When `$CHEZMOI_CONFIG` to be set, chezmoi will act as if it were called
with `chezmoi --config $CHEZMOI_CONFIG`. If both `$CHEZMOI_CONFIG` and
`--config PATH-TO-CONFIG` are provided, the command-line flag wins.
@halostatue halostatue force-pushed the environment-overrides branch from 10600cc to cf9d046 Compare March 8, 2023 18:05
@halostatue
Copy link
Collaborator Author

feat: Find config file with $CHEZMOI_CONFIG

Resolves #2771

When $CHEZMOI_CONFIG to be set, chezmoi will act as if it were called with chezmoi --config $CHEZMOI_CONFIG. If both $CHEZMOI_CONFIG and --config PATH-TO-CONFIG are provided, the command-line flag wins.

doc: Show config vars for global flags

doc: Add JSONC configuration example

doc: Improved information for config vars

@halostatue halostatue changed the title (feat): Add Configuration Path Env Vars feat: Find config file with $CHEZMOI_CONFIG Mar 8, 2023
@halostatue halostatue changed the title feat: Find config file with $CHEZMOI_CONFIG feat: Find config file with $CHEZMOI_CONFIG Mar 8, 2023
@halostatue halostatue changed the title feat: Find config file with $CHEZMOI_CONFIG feat: Find config file with $CHEZMOI_CONFIG Mar 8, 2023
@halostatue halostatue marked this pull request as ready for review March 8, 2023 18:07
@halostatue halostatue requested a review from twpayne March 8, 2023 18:07
@twpayne
Copy link
Owner

twpayne commented Mar 9, 2023

Sorry, I am against this PR.

Currently chezmoi sets a wide range of environment variables for processes that it spawns. All of these variables begin with CHEZMOI.

This PR adds a special environment variable which is the first and only one to be read by chezmoi. This is inconsistent with chezmoi's existing use of environment variables.

I don't like configuration being set via environment variables as they are generally not trivially visible to the user (unlike, say command line arguments). I think the original problem is better solved with a wrapper script or alias.

@halostatue
Copy link
Collaborator Author

OK. Let me see what we can discuss on this:

  1. There are some (I think good) improvements to the documentation. Should I open a PR for those (and rework the bits out which refer to $CHEZMOI_CONFIG)?
  2. Would you accept a PR for improved wrapper script examples that include a $CHEMOI_CONFIG reader so that such a script can be responsive to direnv configuration?

That latter part is sort of what would be important to me—and why I thought this would be a worthwhile, as I regularly have .envrc files that specify AWS_PROFILE specific to that directory for running terraform, for example. That is, I don’t want a czb wrapper function / script; I just want to remember chezmoi apply.

@twpayne
Copy link
Owner

twpayne commented Mar 10, 2023

  • There are some (I think good) improvements to the documentation. Should I open a PR for those (and rework the bits out which refer to $CHEZMOI_CONFIG)?
  • Would you accept a PR for improved wrapper script examples that include a $CHEMOI_CONFIG reader so that such a script can be responsive to direnv configuration?

Yes, both would be fantastic, thank you!

@twpayne
Copy link
Owner

twpayne commented Mar 11, 2023

Superseded by #2843.

@twpayne twpayne closed this Mar 11, 2023
@halostatue halostatue deleted the environment-overrides branch August 17, 2023 02:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 11, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set global flags using environment variables
2 participants