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.

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

FR: Per-repo jj configuration #3684

Open
matts1 opened this issue May 14, 2024 · 9 comments
Open

FR: Per-repo jj configuration #3684

matts1 opened this issue May 14, 2024 · 9 comments
Labels
enhancement New feature or request

Comments

@matts1
Copy link
Collaborator

matts1 commented May 14, 2024

Is your feature request related to a problem? Please describe.
I believe that there are 4 different types of configuration that may be useful for jj:

  • Configuration useful for everybody using jj (default configs)
  • Configuration useful for just me, but for all the repos on my system (~/.config/jj/config.toml)
  • Configuration useful for all users of a particular repo
  • Configuration useful for just me on a particular repo

Sometimes it's quite handy to be able to have per-repo configuration. For example:

  • One repo might be the exception to the rule, and thus I need to come up with a custom revset (eg. immutable_heads())
  • Potential configuration for jj fix and jj lint in the future
  • Pre-upload hook configuration (FR: Generalized hook support #3577)

Describe the solution you'd like
jj already supports layering of configuration via the --config-toml argument.

This is very similar to bazel's .bazelrc files, which support workspace, user, and user-specified rc files (and most workspaces have try-import %workspace%/user.bazelrc at the bottom of their files, so for all intents and purposes, that's supported too).

If it weren't for security concerns, I'd think we should adopt that model in an instant, but since bazel isn't worried about arbitrary code execution (that's literally it's job), we unfortunately have to go further. This is very similar to git hooks and their security model, so I propose the following changes.

  • Read an allowlisted subset of configuration from <repo>/.jj/repo/repo_config.toml
  • Read a full set of configuration from <repo>/.jj/repo/user_config.toml (path open for discussion)

However, to deal with security concerns, we print a warning and ignore the file if either file is tracked by jj. This will prevent someone upstream sending you a file and allowing arbitrary code execution.

The next change we make is very simple. When jj starts, we read <repo>/jj/config.toml (path open for discussion), and, if the file doesn't match <repo>/.jj/repo/repo_config.toml, we print a warning that there is a more up to date configuration, and instruct them to run a command jj install-config or similar.

jj install-config would print a diff, warn them about the security concerns, and ask the user if they want to install the config to the repo (copying the file to <repo>/.jj/repo/repo_config.toml).

Describe alternatives you've considered
We could potentially instead put repo-specific conifguration as its own data format, but I think this is much more flexible and easy to use

Additional context
Add any other context or screenshots about the feature request here.

@kuchta
Copy link

kuchta commented May 14, 2024

jj supports layaring configuration files. See Configuration. What's missing there is --system configuration loaded from something like $(prefix)/etc/jj/config or appropriate for the system for setting e.g. employer/company wide settings, but I suppose it could be solved by company specific distribution of jj

@yuja
Copy link
Collaborator

yuja commented May 14, 2024

Perhaps, the requested feature is "version-controlled" config file? Something like <workspace>/.jjconfig.toml, and will be trusted per (key, value) pair or file content hash.

FWIW, there are security concerns about the current .jj/repo/config.toml, which is tracked by #1595.
#616 (comment) is also related.

@matts1
Copy link
Collaborator Author

matts1 commented May 14, 2024

Yes, the primary purpose of this FR was for version controlled configuration files (I wasnt aware of the .jj/repo/config.toml, so I tried to add in that too, but the primary focus since that seemed trivial).

@poliorcetics
Copy link
Collaborator

If we go the way of a versioned config in a workspace, I would really prefer it to be in <workspace>/.config/jj/... like cargo-nextest does, it makes the root much less cluttered and we don't have any backward compat concerns here. It also make it easily extensible for the future: just add a file in the repo, for example a template one that on clone asks for username/email via a third party tool and creates the actual config, or a project proposing an example config in .config/jj/config.example.toml

@matts1
Copy link
Collaborator Author

matts1 commented May 14, 2024

I much prefer your .config/JJ/config.toml over the example I had. I didn't really like my suggested placement but didn't have a better place to put it.

@joyously
Copy link

So, there needs to be project-wide config (like for hooks), but these can be changed in the life of the project.
Also, user config per project, as each could be a different language (and need different hooks), but they can change in the life of a project.
How are these things usually done? Are they version controlled?
This made me begin to wonder about a future bisect command, to see that running an older version of the code to find the problem could be difficult due to version changes in compilers and config, etc.

@PhilipMetzger PhilipMetzger added the enhancement New feature or request label May 14, 2024
@matts1
Copy link
Collaborator Author

matts1 commented May 15, 2024

So, there needs to be project-wide config (like for hooks), but these can be changed in the life of the project.

Yep

Also, user config per project, as each could be a different language (and need different hooks), but they can change in the life of a project.

I think there are three use cases of user config per project:

  • Project doesn't conform to a normal structure (eg. changing the immutable_heads revset)
  • Project doesn't support jj natively, so doesn't have a builtin config file for jj.
  • Want to override a project default you don't like (eg. modify pre-upload hook to do an additional check).

How are these things usually done? Are they version controlled?

I think we have very little existing use cases to base this off. Version control configuration checked into version control is a relatively novel approach, AFAIK, but jj also has novel concepts which make this more useful.

  • immutable_heads are inherently a per-repo thing (though you can generalize it to make it work for most repos).
  • git hooks are checked in to version control. We need a pre-upload hook in jj (FR: Generalized hook support #3577), but what I'm proposing here is that rather than checking the binary into version control, we check the config into version control, and the config can specify a command.
  • In another thread, @martinvonz said 'Actually, there was some "config-based hooks" effort in Git a while ago (a quick googling led to this).'

This made me begin to wonder about a future bisect command, to see that running an older version of the code to find the problem could be difficult due to version changes in compilers and config, etc.

I think that while that may be a problem, it's mostly unrelated to the config file. I think that in general, hooks are unlikely to change, since hooks should be an alias for what a user might run. If I were to suddenly change the version of rust I require, then my config file would still write "cargo", not "cargo-1.2.3". Similarly, users are generally not expected to add cargo test --foo to their command-line.

I think in general, this problem is solved by appropriate tooling elsewhere. This is simply a general problem of hermeticity, and I think it's outside of the scope of jj. For example, in bazel we use a .bazelversion file containing the version number of bazel we want to use, which is then used by bazelisk to ensure that we always have the correct bazel version for a given piece of source code, and if we checkout an old version, we use the old compiler.

@arxanas
Copy link
Collaborator

arxanas commented Jun 1, 2024

One of the main problems with version control config in version control is that it can affect the behavior of operations potentially outside of the commit that contains the configuration. For example, .gitignore and .gitattributes control whether a file is tracked and how it's merged... which means that if you rebase X onto @, and X has different directives for merging files than @, then you may need to check X for all of its configuration to see what the "correct" behavior for the subsequent rebase ought to be.

The most common example is that a working copy commit may start tracking a given file, and then you may update the .gitignore file such that it excludes that file, but it continues to be tracked by jj for historical reasons. This is an example of checked-in version control configuration differing between the previous and next versions of @, for which the user has to manually resolve the semantic conflict by untracking the newly-ignored file.

I don't recall where the original discussion was, but I think somebody investigated a case for .gitattributes and found that Git just ignores relevant .gitattributes in certain merge/rebase cases when it probably should have been observing them. It might just be fine to ignore this kind of configuration issue in practice (except for the ignoring issue just discussed).

@matts1
Copy link
Collaborator Author

matts1 commented Jun 3, 2024

One of the main problems with version control config in version control is that it can affect the behavior of operations potentially outside of the commit that contains the configuration.

You raise some very good points here, but I believe they should be addressed by this part of my proposal. I'm not proposing that the source of truth be stored in version control (for security reasons, but it happens to address your issues too)

The next change we make is very simple. When jj starts, we read <repo>/jj/config.toml (path open for discussion), and, if the file doesn't match <repo>/.jj/repo/repo_config.toml, we print a warning that there is a more up to date configuration, and instruct them to run a command jj install-config or similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants