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: Resolve many SSH issues by having networked jj git commands shell out to git #4979

Open
arxanas opened this issue Nov 26, 2024 · 5 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@arxanas
Copy link
Contributor

arxanas commented Nov 26, 2024

Is your feature request related to a problem? Please describe.

Many jj git commands fail on less common setups, oftentimes because libgit2 doesn't support certain features (see additional context).

libgit2 itself uses libssh2 rather than OpenSSH, so it often doesn't behave the way that users would expect with their current git/ssh usage.

We've discussed having jj git commands literally shell out to git, rather than using the libgit2 implementation, most recently in #4937 (comment). I'm opening this issue to justify and track that proposal.

Describe the solution you'd like

Change jj git commands to execute git directly where appropriate.

This is not risk-free. For example, git-branchless does this and instead has issues like git sync --pull, git submit hang indefinitely when asking for password (arxanas/git-branchless#433) (note that it tries to run git in the background and communicate via stdin/stdout). But there are substantially fewer networking-related issues there, so it seems like a better default state of affairs for most users.

Describe alternatives you've considered

  • Continue using libgit2+libssh2: It seems like there's a significant number of issues caused by lack of support in libgit2, to the point that it impedes new and existing users, and we don't have the bandwidth or expertise to address them all.
  • Switch to Gitoxide: I don't think Gitoxide currently supports all of these workflows, and might not in the future. Gitoxide even shells out to git at present for certain use-cases (reading certain config?).

Additional context

Here's my enumeration of issues that might be resolved or resolveable by shelling out to Git. It would be nice if we could mostly solve them with a single mechanism!

SSH:

Clone/fetch/push/pull:

@arxanas arxanas added the enhancement New feature or request label Nov 26, 2024
@arxanas arxanas added the help wanted Extra attention is needed label Dec 19, 2024
@martinvonz martinvonz pinned this issue Dec 19, 2024
@arxanas arxanas changed the title FR: Have networked jj git commands shell out to git FR: Resolve many SSH issues by having networked jj git commands shell out to git Dec 21, 2024
@gulbanana
Copy link
Contributor

It would certainly fix some issues. Do we have any concerns about inconsistency between git-backend ops performed by git itself and similar operations performed by jj's other commands?

@arxanas
Copy link
Contributor Author

arxanas commented Jan 2, 2025

It would certainly fix some issues. Do we have any concerns about inconsistency between git-backend ops performed by git itself and similar operations performed by jj's other commands?

There are some existing issues, such as that some Git tooling doesn't handle commits with empty messages well. For that reason, jj git push checks and forbids pushing commits with empty messages. But I don't anticipate any new issues that would arise from using libgit2 vs git to push.

bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - it is using a new feature flag `shell` to switch on to shelling out.
   this doesn't seem the best approach, and it would be great to get
   some feedback on what would be best. A flag on jj git + adding it to
   the jj config seems to be a good enough idea

Testing:

Run the rust tests:
```
$ cargo test # checks we didn't screw up the baseline
$ cargo test --features=shell # test the shelling out
```

Build with shell-out enabled:
```
$ cargo build --features=shell
```

Clone a private repo:
```
$ jj git clone <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push -b <branch>
```
@arxanas
Copy link
Contributor Author

arxanas commented Jan 2, 2025

As a potential risk, @emilazy pointed this out on Discord:

another thing that probably needs consideration is packaging – getting git on PATH is no burden for macOS or Linux users, but I imagine we probably have prospective Windows users who don't want to install Git Bash or similar…?

So we should be sure to think about the workflow+UX for Windows users, or anyone else who doesn't happen to have git on their PATH for whatever reason.

  • I recall at least one situation where a Windows user was excited to use jj specifically because it didn't require git, and then were disappointed that something in their workflow actually did need git (maybe? I don't remember exactly what happened).
  • A compiled-to-WebAssembly distribution might not have git available, although in such a context, it seems likely that we wouldn't support pushing anyways.
  • There's some discussion in that Discord thread about Nix users, but I would assume for now that Nix users know how to handle it themselves.

@emilazy
Copy link
Contributor

emilazy commented Jan 2, 2025

Just to record what I said on the Discord, I think that it’s basically no issue for anything but Windows, and for a seamless Windows experience we could build our own Git binaries in CI and ship them with Jujutsu (in the limit, by using build.rs fun to literally embed a stripped‐down Git main() in our jj.exe and shelling out to ourselves) – but I think this isn’t worth worrying about until we work on a polished Windows installation experience in general.

bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 2, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
@gulbanana
Copy link
Contributor

I think "for git interop, install git so that jj can use it to connect to remotes" is reasonable for now, even on Windows. We could make it a little better by trying some hardcoded locations in case of installs that haven't set PATH.

bsdinis added a commit to bsdinis/jj that referenced this issue Jan 3, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 4, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 4, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes shelling
out to `git` for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
shell = true
```

Or via a CLI argument, e.g.: `jj git clone --shell <url>`

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build with shell-out enabled:
```
$ cargo build
```

Clone a private repo:
```
$ jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ jj describe -m 'test commit'
$ jj git push --shell -b <branch>
```
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 4, 2025
Reasoning:

`jj` fails to push/fetch over ssh depending on the system.
Issue jj-vcs#4979 lists over 20 related issues on this and proposes spawning
a `git` subprocess for tasks related to the network (in fact, just push/fetch
are enough).

This PR implements this.
Users can either enable shelling out to git in a config file:

```toml
[git]
subprocess = true
```

Implementation Details:

This PR implements shelling out to `git` via `std::process::Command`.
There are 2 sharp edges with the patch:
 - it relies on having to parse out git errors to match the error codes
   (and parsing git2's errors in one particular instance to match the
   error behaviour). This seems mostly unavoidable

 - to ensure matching behaviour with git2, the tests are maintained across the
   two implementations. This is done using test_case, as with the rest
   of the codebase

Testing:

Run the rust tests:
```
$ cargo test
```

Build:
```
$ cargo build
```

Clone a private repo:
```
$ path/to/jj git clone --shell <REPO_SSH_URL>
```

Create new commit and push
```
$ echo "TEST" > this_is_a_test_file.txt
$ path/to/jj describe -m 'test commit'
$ path/to/jj git push --shell -b <branch>
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants