Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add OpenSSH support#6617

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ethomson merged 21 commits intomainfromethomson/openssh
Aug 31, 2023
Merged

Add OpenSSH support#6617

ethomson merged 21 commits intomainfromethomson/openssh
Aug 31, 2023

Conversation

ethomson
Copy link
Member

Provide a smart transport that executesssh ... - this adds a mechanism for executing processes from libgit2, and uses it to invokessh and deal with the output.

Dr-Emann, necauqua, bergkvist, extrawurst, mimame, melMass, Ericson2314, and Frestein reacted with hooray emoji
bergkvist added a commit to bergkvist/git2-rs that referenced this pull requestAug 3, 2023
@bergkvist
Copy link

bergkvist commentedAug 3, 2023
edited
Loading

I'm getting a segfault trying to use it:

$ git clone git@github.com:bergkvist/jj && cd jj$ cargo install --path=.$ ~/.cargo/bin/jj init --git-repo=.$ ~/.cargo/bin/jj git fetchSegmentation fault

I forked/modified the following repos (Cargo.toml/submodule links only) to use this patch:

I'm getting the segfault on both macOS and Linux

@ethomson
Copy link
MemberAuthor

Neat, thanks for trying this out. Can you get a core or stack trace?

@bergkvist
Copy link

Valgrind:

valgrind ./target/debug/jj git fetch==54547== Memcheck, a memory error detector==54547== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.==54547== Using Valgrind-3.19.0 and LibVEX; rerun with -h for copyright info==54547== Command: ./target/debug/jj git fetch==54547====54547== Invalid read of size 8==54547==    at 0x1142164: git_transport_smart (smart.c:526)==54547==    by 0x111AD1B: git_transport_new (transport.c:134)==54547==    by 0x1103E77: git_remote_connect_ext (remote.c:960)==54547==    by 0x11049CB: connect_or_reset_options (remote.c:1254)==54547==    by 0x1104D5F: git_remote_download (remote.c:1339)==54547==    by 0x82E8BF: git2::remote::Remote::download (remote.rs:249)==54547==    by 0x87951B: jj_lib::git::fetch (git.rs:675)==54547==    by 0x46AD5F: jj_cli::commands::git::cmd_git_fetch::{{closure}} (git.rs:303)==54547==    by 0x469683: jj_cli::commands::git::with_remote_callbacks (git.rs:522)==54547==    by 0x3FBF23: jj_cli::commands::git::cmd_git_fetch (git.rs:302)==54547==    by 0x3F31D7: jj_cli::commands::git::cmd_git (git.rs:1027)==54547==    by 0x276337: jj_cli::commands::run_command (mod.rs:3743)==54547==  Address 0x10 is not stack'd, malloc'd or (recently) free'd==54547====54547== Jump to the invalid address stated on the next line==54547==    at 0x0: ???==54547==  Address 0x0 is not stack'd, malloc'd or (recently) free'd==54547====54547====54547== Process terminating with default action of signal 11 (SIGSEGV)==54547==  Bad permissions for mapped region at address 0x0==54547==    at 0x0: ???==54547====54547== HEAP SUMMARY:==54547==     in use at exit: 2,480,221 bytes in 23,448 blocks==54547==   total heap usage: 336,841 allocs, 313,393 frees, 22,545,464 bytes allocated==54547====54547== LEAK SUMMARY:==54547==    definitely lost: 0 bytes in 0 blocks==54547==    indirectly lost: 0 bytes in 0 blocks==54547==      possibly lost: 182,032 bytes in 1,613 blocks==54547==    still reachable: 2,298,189 bytes in 21,835 blocks==54547==         suppressed: 0 bytes in 0 blocks==54547== Rerun with --leak-check=full to see details of leaked memory==54547====54547== For lists of detected and suppressed errors, rerun with: -s==54547== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)Segmentation fault

@bergkvist
Copy link

Commenting out thet->wrapped->free(t->wrapped); in smart.c:526 - I instead get:

Error: cannot create SSH transport; library was built without SSH support; class=Invalid (3)

@ethomson
Copy link
MemberAuthor

Oops, that's definitely a mistake. It looks like I haven't built that branchwithout any SSH support in a while. Thanks for catching that.

You'll need to configure libgit2 withcmake -DUSE_SSH=exec <other args> when you compile it. Sorry, I should have mentioned that upthread.

@bergkvist
Copy link

I had to do this to get it working:bergkvist/git2-rs@9a4d232 (git2-rs doesn't use cmake)

I would love for this to get merged into libgit2.

@ethomson
Copy link
MemberAuthor

I would love for this to get merged into libgit2.

It'll happen - I think that I just want to give plenty of time for feedback. Doing anexec could be new and scary for some people, even if it's only conditionally compiled in.

We may want to support SSH but with a different provider that is notlibssh2. Add GIT_SSH to indicate that we have some inbuilt SSH supportand GIT_SSH_LIBSSH2 to indicate that support is via libssh2. This issimilar to how we support GIT_HTTPS and GIT_OPENSSL, for example.
We can now use the `git_process` class to invoke OpenSSH and use it asan SSH transport. This may be preferred over libssh2 for a variety ofcallers.
We can't reliably detect SIGPIPE on close because of platformdifferences. Track `pid` and send `SIGTERM` to a function and ensurethat we can detect it.
There are no custom callbacks for OpenSSH; don't test them.
Now that we (may) exec a child process to do ssh, we don't want valgrindreporting on that. Suppress children in valgrind runs.
A transport may want to validate that it's in a sane state; whenflushing on close, don't assume that we're doing an upload-pack; sendthe correct direction.
Instead of "early EOF", provide information on _when_ we're seeing theEOF for debugging.
Suppress SIGPIPEs during writes to our piped process. On single-threadedapplications, this is as simple as ignoring the signal. But since thisis process-wide, on multi-threaded applications, we need to use somecumbersome `pthread_sigmask` manipulation.Thanks tohttps://www.doof.me.uk/2020/09/23/sigpipe-and-how-to-ignore-it/andhttp://www.microhowto.info:80/howto/ignore_sigpipe_without_affecting_other_threads_in_a_process.html
Provide a mechanism for callers to read from stderr.
Provide more user-friendly error messages in smart protocol negotiationfailures.
Don't capture stderr, optimize for the CLI case.
Provide both cmdline-style handling (passing it to the shell on POSIX,or directly to CreateProcess on win32) and execv style (passing itdirectly to execv on POSIX, and mangling it into a single command-lineon win32).
Callers can specify the ssh command to invoke using `core.sshcommand` orthe `GIT_SSH` environment variable. This is useful for specifyingalternate configuration, and is particularly useful for our testingenvironment.
This helped when troubleshooting issues running the `ci/test.sh` scriptlocally.
Handle custom paths for OpenSSH.
@ethomsonethomson merged commitf51e70d intomainAug 31, 2023
@ethomsonethomson deleted the ethomson/openssh branchAugust 31, 2023 08:34
edolstra added a commit to edolstra/nix that referenced this pull requestNov 14, 2023
Seelibgit2/libgit2#6617. This ensures that weget support for ~/.ssh/config, known_hosts etc.
bnjmnt4n added a commit to bnjmnt4n/git2-rs that referenced this pull requestMar 1, 2024
This commit changes the original `ssh` feature into two new ones:`ssh-libssh2` and `ssh-openssh`. By default, the `ssh-libssh2` featureis enabled for backwards compatibility.To use OpenSSH instead, the following listing in `Cargo.toml` can beused:    git2-rs = { version = "...", default-features = false, features = ["https", "ssh-openssh"] }Note thatlibgit2/libgit2#6617 has not actuallybeen released in an official libgit2 version, so the prior commit pulledin the latest commit from `main`.
bnjmnt4n added a commit to bnjmnt4n/git2-rs that referenced this pull requestMar 1, 2024
This commit changes the original `ssh` feature into two new ones:`ssh-libssh2` and `ssh-openssh`. By default, the `ssh-libssh2` featureis enabled for backwards compatibility.To use OpenSSH instead, the following listing in `Cargo.toml` can beused:    git2-rs = { version = "...", default-features = false, features = ["https", "ssh-openssh"] }Note thatlibgit2/libgit2#6617 has not actuallybeen released in an official libgit2 version, so the prior commit pulledin the latest commit from `main`.Closesrust-lang#1028.
@bnjmnt4n
Copy link

Hey, whilst looking through this PR, I realized that there are some instances whereGIT_SSH_MEMORY_CREDENTIALS was not renamed toGIT_SSH_LIBSSH2_MEMORY_CREDENTIALS. Is that a mistake?

bnjmnt4n added a commit to bnjmnt4n/git2-rs that referenced this pull requestMar 2, 2024
This commit changes the original `ssh` feature into two new ones:`ssh-libssh2` and `ssh-openssh`. By default, the `ssh-libssh2` featureis enabled for backwards compatibility.To use OpenSSH instead, the following listing in `Cargo.toml` can beused:    git2-rs = { version = "...", default-features = false, features = ["https", "ssh-openssh"] }Note thatlibgit2/libgit2#6617 has not actuallybeen released in an official libgit2 version, so the prior commit pulledin the latest commit from `main`.Closesrust-lang#1028.
bnjmnt4n added a commit to bnjmnt4n/git2-rs that referenced this pull requestMar 2, 2024
This commit changes the original `ssh` feature into two new ones:`ssh-libssh2` and `ssh-openssh`. By default, the `ssh-libssh2` featureis enabled for backwards compatibility.To use OpenSSH instead, the following listing in `Cargo.toml` can beused:    git2-rs = { version = "...", default-features = false, features = ["https", "ssh-openssh"] }Note thatlibgit2/libgit2#6617 has not actuallybeen released in an official libgit2 version, so the prior commit pulledin the latest commit from `main`.Closesrust-lang#1028.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

3 participants
@ethomson@bergkvist@bnjmnt4n

[8]ページ先頭

©2009-2025 Movatter.jp