Skip to content

Commit

Permalink
cli: git: store absolute remote path in config file
Browse files Browse the repository at this point in the history
The "git" CLI chdir()s to the work tree root, so paths in config file are
usually resolved relative to the workspace root. OTOH, jj doesn't modify the
process environment, so libgit2 resolves remote paths relative to cwd, not to
the workspace root. To mitigate the problem, this patch makes "jj git remote"
sub commands to store resolved path in .git/config. It would be nice if we can
reconfigure in-memory git2 remote object to use absolute paths (or set up
in-memory named remote without writing a config file), but there's no usable
API afaik.

This behavior is different from "git remote add"/"set-url". I don't know the
rationale, but these commands don't resolve relative paths, whereas "git clone"
writes resolved path to .git/config. I think it's more consistent to make all
"jj git" sub commands resolve relative paths.
  • Loading branch information
yuja committed Jan 12, 2025
1 parent 20b3d02 commit 89e0a70
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
* In `jj config list` template, `value` is now typed as `ConfigValue`, not as
`String` serialized in TOML syntax.

* `jj git remote add`/`set-url` now converts relative Git remote path to
absolute path.

* Upgraded `scm-record` from v0.4.0 to v0.5.0. See release notes at
<https://github.com/arxanas/scm-record/releases/tag/v0.5.0>.

Expand Down
2 changes: 2 additions & 0 deletions cli/src/commands/git/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ use crate::ui::Ui;
#[derive(clap::Args, Clone, Debug)]
pub struct GitCloneArgs {
/// URL or path of the Git repo to clone
///
/// Local path will be resolved to absolute form.
#[arg(value_hint = clap::ValueHint::DirPath)]
source: String,
/// Specifies the target directory for the Jujutsu repository clone.
Expand Down
9 changes: 7 additions & 2 deletions cli/src/commands/git/remote/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use jj_lib::repo::Repo;

use crate::cli_util::CommandHelper;
use crate::command_error::CommandError;
use crate::git_util::absolute_git_url;
use crate::git_util::get_git_repo;
use crate::ui::Ui;

Expand All @@ -25,7 +26,10 @@ use crate::ui::Ui;
pub struct GitRemoteAddArgs {
/// The remote's name
remote: String,
/// The remote's URL
/// The remote's URL or path
///
/// Local path will be resolved to absolute form.
#[arg(value_hint = clap::ValueHint::DirPath)]
url: String,
}

Expand All @@ -37,6 +41,7 @@ pub fn cmd_git_remote_add(
let workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo();
let git_repo = get_git_repo(repo.store())?;
git::add_remote(&git_repo, &args.remote, &args.url)?;
let url = absolute_git_url(command.cwd(), &args.url)?;
git::add_remote(&git_repo, &args.remote, &url)?;
Ok(())
}
9 changes: 7 additions & 2 deletions cli/src/commands/git/remote/set_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use jj_lib::repo::Repo;
use crate::cli_util::CommandHelper;
use crate::command_error::CommandError;
use crate::complete;
use crate::git_util::absolute_git_url;
use crate::git_util::get_git_repo;
use crate::ui::Ui;

Expand All @@ -28,7 +29,10 @@ pub struct GitRemoteSetUrlArgs {
/// The remote's name
#[arg(add = ArgValueCandidates::new(complete::git_remotes))]
remote: String,
/// The desired url for `remote`
/// The desired URL or path for `remote`
///
/// Local path will be resolved to absolute form.
#[arg(value_hint = clap::ValueHint::DirPath)]
url: String,
}

Expand All @@ -40,6 +44,7 @@ pub fn cmd_git_remote_set_url(
let workspace_command = command.workspace_helper(ui)?;
let repo = workspace_command.repo();
let git_repo = get_git_repo(repo.store())?;
git::set_remote_url(&git_repo, &args.remote, &args.url)?;
let url = absolute_git_url(command.cwd(), &args.url)?;
git::set_remote_url(&git_repo, &args.remote, &url)?;
Ok(())
}
10 changes: 8 additions & 2 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,8 @@ The Git repo will be a bare git repo stored inside the `.jj/` directory.
###### **Arguments:**
* `<SOURCE>` — URL or path of the Git repo to clone
Local path will be resolved to absolute form.
* `<DESTINATION>` — Specifies the target directory for the Jujutsu repository clone. If not provided, defaults to a directory named after the last component of the source URL. The full directory path will be created if it doesn't exist
###### **Options:**
Expand Down Expand Up @@ -1203,7 +1205,9 @@ Add a Git remote
###### **Arguments:**
* `<REMOTE>` — The remote's name
* `<URL>` — The remote's URL
* `<URL>` — The remote's URL or path
Local path will be resolved to absolute form.
Expand Down Expand Up @@ -1249,7 +1253,9 @@ Set the URL of a Git remote
###### **Arguments:**
* `<REMOTE>` — The remote's name
* `<URL>` — The desired url for `remote`
* `<URL>` — The desired URL or path for `remote`
Local path will be resolved to absolute form.
Expand Down
25 changes: 25 additions & 0 deletions cli/tests/test_git_remotes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

use std::fs;
use std::path::PathBuf;

use crate::common::TestEnvironment;

Expand Down Expand Up @@ -144,6 +145,30 @@ fn test_git_remote_set_url() {
"###);
}

#[test]
fn test_git_remote_relative_path() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
let repo_path = test_env.env_root().join("repo");

// Relative path using OS-native separator
let path = PathBuf::from_iter(["..", "native", "sep"]);
test_env.jj_cmd_ok(
&repo_path,
&["git", "remote", "add", "foo", path.to_str().unwrap()],
);
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]);
insta::assert_snapshot!(stdout, @"foo $TEST_ENV/native/sep");

// Relative path using UNIX separator
test_env.jj_cmd_ok(
test_env.env_root(),
&["-Rrepo", "git", "remote", "set-url", "foo", "unix/sep"],
);
let stdout = test_env.jj_cmd_success(&repo_path, &["git", "remote", "list"]);
insta::assert_snapshot!(stdout, @"foo $TEST_ENV/unix/sep");
}

#[test]
fn test_git_remote_rename() {
let test_env = TestEnvironment::default();
Expand Down

0 comments on commit 89e0a70

Please sign in to comment.