Skip to content

fix(jj): load user config so committer signature is set#202

Open
madmo wants to merge 1 commit into
cococonscious:feature/jj-supportfrom
madmo:mbitsch/push-mpkqopyqxsww
Open

fix(jj): load user config so committer signature is set#202
madmo wants to merge 1 commit into
cococonscious:feature/jj-supportfrom
madmo:mbitsch/push-mpkqopyqxsww

Conversation

@madmo

@madmo madmo commented May 12, 2026

Copy link
Copy Markdown

Previously, jj_load_repo used StackedConfig::with_defaults() which only contains empty strings for user.name and user.email. When rewrite_commit is called, jj-lib sets the committer field from settings.signature(), resulting in an empty committer that prevents pushing to git remotes.

Port the config-loading helpers from the jj CLI (env_base_layer, env_overrides_layer, UnresolvedConfigEnv::resolve) and wire them together so that the StackedConfig is built with the same layer ordering as the jj CLI: Default, EnvBase, User, Repo, EnvOverrides.

@codecov

codecov Bot commented May 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
src/lib/vcs.rs 90.81% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@madmo madmo force-pushed the mbitsch/push-mpkqopyqxsww branch from ec9c403 to 312b45a Compare May 13, 2026 12:14
Comment thread src/lib/vcs.rs
///
/// Ported from jj cli/src/config.rs `env_base_layer`.
#[cfg(feature = "jj")]
fn jj_env_base_layer() -> jj_lib::config::ConfigLayer {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please add some tests for the new methods.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sure, took me some time, but now the new functionality has some tests. Thanks for reviewing!

@madmo madmo force-pushed the mbitsch/push-mpkqopyqxsww branch 3 times, most recently from 6e00ea4 to e3fe0be Compare June 5, 2026 15:06

@cococonscious cococonscious left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sorry for only bringing this up now, but you should probably improve the license attribution, as how it is right now it's most likely not sufficient.

Previously, jj_load_repo used StackedConfig::with_defaults() which only
contains empty strings for user.name and user.email. When rewrite_commit
is called, jj-lib sets the committer field from settings.signature(),
resulting in an empty committer that prevents pushing to git remotes.

Port the config-loading helpers from the jj CLI (env_base_layer,
env_overrides_layer, UnresolvedConfigEnv::resolve) and wire them
together so that the StackedConfig is built with the same layer
ordering as the jj CLI: Default, EnvBase, User, Repo, EnvOverrides.
@madmo madmo force-pushed the mbitsch/push-mpkqopyqxsww branch from e3fe0be to 138fd01 Compare June 16, 2026 20:46
@madmo

madmo commented Jun 16, 2026

Copy link
Copy Markdown
Author

Sorry for only bringing this up now, but you should probably improve the license attribution, as how it is right now it's most likely not sufficient.

Good point, the attribution part was really on the minimal side. Now it should be enough to cover the requirements of the Apache-2.0 license.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants