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

SSH login with gpg keycard backed ssh key and pinentry-curses for pin fails #4386

Open
necauqua opened this issue Sep 4, 2024 · 0 comments
Labels
🐛bug Something isn't working

Comments

@necauqua
Copy link
Contributor

necauqua commented Sep 4, 2024

Description

I know that this is a contrived setup (hence it probably wasn't reported), just making sure this is tracked.

Steps to Reproduce the Problem

  1. Have a yubikey or similar
  2. Configure a gpg auth subkey to be used for SSH and export it to the hw key
  3. Configure gpg to use pinentry-ncurses for card pin

Expected Behavior

When you do fetch/push, pinentry should ask for the pin and then everything should work.

Actual Behavior

Instead you get the classic Error: authentication required but no callback set; class=Ssh (23); code=Auth (-16).

If you ssh into something once for the pin to get cached it starts working (without asking for pin, obviously).

When it was not pinentry-curses but whatever was the default it worked fine, asking for the pin the one time with a separate dialog window.

And the most interesting thing is that gpg signing works! When you snapshot or whatever for the first time after boot it asks for the pin with pinentry and works just fine. Probably because for gpg we just call the gpg binary and for ssh it's another libssh issue - so this likely may be fixed by the openssh build.

Specifications

  • Platform: NixOS
  • Version: 0.20.0
@PhilipMetzger PhilipMetzger added the 🐛bug Something isn't working label Sep 4, 2024
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 10, 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>
```

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 10, 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>
```

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 10, 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>
```

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 11, 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>
```

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis pushed a commit to bsdinis/jj that referenced this issue Jan 11, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis pushed a commit to bsdinis/jj that referenced this issue Jan 11, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 11, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 11, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 12, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 13, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
bsdinis added a commit to bsdinis/jj that referenced this issue Jan 13, 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>
```

<!--
There's no need to add anything here, but feel free to add a personal message.
Please describe the changes in this PR in the commit message(s) instead, with
each commit representing one logical change. Address code review comments by
rewriting the commits rather than adding commits on top. Use force-push when
pushing the updated commits (`jj git push` does that automatically when you
rewrite commits). Merge the PR at will once it's been approved. See
https://github.com/jj-vcs/jj/blob/main/docs/contributing.md for details.
Note that you need to sign Google's CLA to contribute.
-->

Issues Closed

With a grain of salt, but most of these problems should be fixed (or at least checked if they are fixed). They are the ones listed in jj-vcs#4979 .

SSH:
- jj-vcs#63
- jj-vcs#440
- jj-vcs#1455
- jj-vcs#1507
- jj-vcs#2931
- jj-vcs#2958
- jj-vcs#3322
- jj-vcs#4101
- jj-vcs#4333
- jj-vcs#4386
- jj-vcs#4488
- jj-vcs#4591
- jj-vcs#4802
- jj-vcs#4870
- jj-vcs#4937
- jj-vcs#4978
- jj-vcs#5120
- jj-vcs#5166

Clone/fetch/push/pull:
- jj-vcs#360
- jj-vcs#1278
- jj-vcs#1957
- jj-vcs#2295
- jj-vcs#3851
- jj-vcs#4177
- jj-vcs#4682
- jj-vcs#4719
- jj-vcs#4889
- jj-vcs#5147
- jj-vcs#5238

Notable Holdouts:
 - Interactive HTTP authentication (jj-vcs#401, jj-vcs#469)
 - libssh2-sys dependency on windows problem (can only be removed if/when we get rid of libgit2): jj-vcs#3984
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants