git: Always use OpenSSL on win32

[HybridEidolon]

By default, libssh2 uses Windows Cryptography Next Generation when targeting win32. The wincng backend does not support ED25519, which is a widely-used algorithm among git hosting services, and in some cases may be the only option on remotes in certain configurations.

This change adds the feature openssl-on-win32, enabling the same one in libssh2-sys, and enables it in the CI release flow, ensuring that release builds are able to communicate with hosts using ED25519.

Fixes #3322

Checklist

If applicable:

• [x] I have updated CHANGELOG.md
• [x] I have updated the documentation (README.md, docs/, demos/)
• [ ] (N/A) I have updated the config schema (cli/src/config-schema.json)
• [ ] (N/A) I have added tests to cover my changes

[google-cla[bot]]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[HybridEidolon]

I updated my CLA signature.

[jhechtf]

I use windows, let me pull this down and give it a look.

EDIT: Build fails for me, is there something more I need to do?

[steveklabnik]

This change necessitates Windows users building jj to install openssl via vcpkg for the target triple x64-windows-static-md when building for MSVC, or otherwise have OpenSSL available at build time.

FWIW, this can be a huge pain. I really, really would prefer not to use openssl unless I have to, and I don't need the features in this PR. I think it might be nicer to have the option to use openssl if you want these extra features for some extra pain in building, but that's just my own personal opinion :)

[khionu]

Maybe this could be a feature flag?

[HybridEidolon]

Maybe this could be a feature flag?

It is technically already one, but enabled during release builds in CI.

This change necessitates Windows users building jj to install openssl via vcpkg for the target triple x64-windows-static-md when building for MSVC, or otherwise have OpenSSL available at build time.

FWIW, this can be a huge pain. ...

I hadn't actually updated the PR text yet; it now uses the vendored-openssl feature that's already turned on for release builds for Windows and macOS, so it's not disruptive and vcpkg is only needed if you specifically enable openssl-on-win32 and disable vendored-openssl.

To my knowledge, OpenSSL is only used for the cryptographic primitives in libssh2 and not for TLS relative stuff.

[jhechtf]

Clarifying question for me; would this allow me to use jj git push on windows now? I can't do it because my ID key for github is an ed2559 key so i have to manually do git push and it's a bit annoying.

[HybridEidolon]

Clarifying question for me; would this allow me to use jj git push on windows now? I can't do it because my ID key for github is an ed2559 key so i have to manually do git push and it's a bit annoying.

Yes. The issue goes both for your own key and the host's, I believe.

[Zoybean]

I'm on windows, tried using this but I can't get openssl-sys to compile. It needs the openssl libs and headers to be available, but I think I only have the libs as part of my git-for-windows install.

(looks like github ate my last comment. this was 2nd attempt)

[HybridEidolon]

I'm on windows, tried using this but I can't get openssl-sys to compile. It needs the openssl libs and headers to be available, but I think I only have the libs as part of my git-for-windows install.

You can use the vendored-openssl feature to compile it statically. This is normally turned on in CI for release builds already.

[farnoy]

I tried testing it with $ cargo install --locked --bin jj --git https://github.com/HybridEidolon/jj --branch push-pklwzruzswky jj-cli --features vendored-openssl,openssl-on-win32 --force but it fails on vcpkg when compiling openssl-sys. I thought this was supposed to work without having openssl installed on the system?

[HybridEidolon]

but it fails on vcpkg when compiling openssl-sys. I thought this was supposed to work without having openssl installed on the system?

It is; the vendored-openssl feature is never supposed to invoke vcpkg. Per the openssl crate documentation,

If the vendored Cargo feature is enabled, the openssl-src crate will be used to compile and statically link to a copy of OpenSSL. The build process requires a C compiler, perl (and perl-core), and make.

[Zoybean]

the vendored-openssl feature is never supposed to invoke vcpkg. Per the openssl crate documentation,

Not sure if having the log helps, but Installing the latest version (c59e79db) with: cargo install --path cli --locked --features packaging,openssl-on-win32,vendored-openssl, I get the following:

error: failed to run custom build command for `openssl-sys v0.9.101`

Caused by:
  process didn't exit successfully: `C:\..redacted..\jj\target\release\build\openssl-sys-88af3904954fd47d\build-script-main` (exit code: 101)
  --- stdout
  cargo:rerun-if-env-changed=X86_64_PC_WINDOWS_MSVC_OPENSSL_LIB_DIR
  X86_64_PC_WINDOWS_MSVC_OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_LIB_DIR
  OPENSSL_LIB_DIR unset
  cargo:rerun-if-env-changed=X86_64_PC_WINDOWS_MSVC_OPENSSL_INCLUDE_DIR
  X86_64_PC_WINDOWS_MSVC_OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_INCLUDE_DIR
  OPENSSL_INCLUDE_DIR unset
  cargo:rerun-if-env-changed=X86_64_PC_WINDOWS_MSVC_OPENSSL_DIR
  X86_64_PC_WINDOWS_MSVC_OPENSSL_DIR unset
  cargo:rerun-if-env-changed=OPENSSL_DIR
  OPENSSL_DIR unset
  note: vcpkg did not find openssl: Could not find Vcpkg tree: No vcpkg installation found. Set the VCPKG_ROOT environment variable or run 'vcpkg integrate install'

  --- stderr
  thread 'main' panicked at C:\..redacted..\.cargo\registry\src\index.crates.io-6f17d22bba15001f\openssl-sys-0.9.101\build\find_normal.rs:190:5:


  Could not find directory of OpenSSL installation, and this `-sys` crate cannot
  proceed without this knowledge. If OpenSSL is installed and this crate had
  trouble finding it,  you can set the `OPENSSL_DIR` environment variable for the
  compilation process.

  Make sure you also have the development packages of openssl installed.
  For example, `libssl-dev` on Ubuntu or `openssl-devel` on Fedora.

  If you're in a situation where you think the directory *should* be found
  automatically, please open a bug at https://github.com/sfackler/rust-openssl
  and include information about your system as well as this message.

  $HOST = x86_64-pc-windows-msvc
  $TARGET = x86_64-pc-windows-msvc
  openssl-sys = 0.9.101


  It looks like you're compiling for MSVC but we couldn't detect an OpenSSL
  installation. If there isn't one installed then you can try the rust-openssl
  README for more information about how to download precompiled binaries of
  OpenSSL:

  https://github.com/sfackler/rust-openssl#windows


  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

[HybridEidolon]

Not sure if having the log helps, but Installing the latest version (c59e79d) with: cargo install --path cli --locked --features packaging,openssl-on-win32,vendored-openssl, I get the following:

Could you try building it after checking out the commit with cargo build instead? I'm not sure what is happening here, but I am building for MSVC locally in a clean context and I don't see this at all.

[Zoybean]

building cli at c59e79d:

• cargo build --locked --features packaging,openssl-on-win32,vendored-openssl fails
• cargo build --locked --features openssl-on-win32,vendored-openssl fails
• cargo build --locked --features openssl-on-win32 fails
• cargo build --locked and no features succeeds of course

All the failures had what appeared to be the same error message, though I didn't bother to run them through diff. (I forget why I added the packaging feature in the first place but it looks like it doesn't affect this error)

[HybridEidolon]

Oh boy, building openssl-sys with the vendored feature only seems to work under msys2 shell on Windows.

Huh...

[gulbanana]

Aha. I was also using msys2 - the version that comes with Git for Windows as "Git Bash".

[Zoybean]

If it helps, I've been building under nushell, but I get the same failure when running in git bash. I don't have an explicit msys2 install.

[HybridEidolon]

I guess I could alter this to not run in CI release packaging, since it does work, it just requires a specific build environment? All it does is forward the feature down to the libssh-sys crate ultimately. I do find it's the only way to make jj useful in my specific use case.

[jhechtf]

I would definitely like for jj git push to work on Windows for me, but AFAIK the only way to install it is from source, and I can never seem to get this pr to build.

[ErichDonGubler]

I was able to build on Windows 11 locally without using MSYS2 using these commands (in Nushell/sh-ish syntax, rather than cmd or PowerShell, but it should work in cmd equivalents, too):

# Get a copy of `vcpkg.exe`.
$ gh repo clone microsoft/vcpkg $VCPKG_SRC_DIR
<snip>
$ mkdir $VCPKG_BIN_DIR
$ cd $VCPKG_BIN_DIR
$ $VCPKG_SRC_DIR\bootstrap-vcpkg.bat # downloads `vcpkg.exe`
<snip>

# Install OpenSSL.
$ .\vcpkg.exe install openssl:x64-windows-static-md
<snip>

# Test a build!
$ VCPKG_ROOT=$VCPKG_BIN_DIR cargo run --release --features openssl-on-win32 -- --version
<snip>
jj 0.17.1-89fc5743d112c49d60d15e1c8684dfa9e3d242e4

…so this build seems reproducible, just not without some expertise.

We may be able to use vcpkg in CI with an action like https://github.com/marketplace/actions/run-vcpkg.

[ErichDonGubler]

Hmm, I get this seemingly incorrect hint while testing jj git push using the build I mentioned in my previous comment:

Hint: Jujutsu uses libssh2, which doesn't respect ~/.ssh/config. Does `ssh -F /dev/null` to the host work?

I say "incorrect" because this build/PR actually uses OpenSSH under the hood, right?

---

Full output, for the curious.

I had tried to push to this PR, which I expected to fail with a permissions error:

VCPKG_ROOT=E:\vcpkg cargo run --release --features openssl-on-win32 -- git push
    Finished `release` profile [optimized] target(s) in 0.33s
     Running `target\release\jj.exe git push`
Branch changes to push to origin:
  Add branch push-pklwzruzswky to 89fc5743d112
Error: ERROR: Permission to martinvonz/jj.git denied to ErichDonGubler.
; class=Ssh (23); code=Eof (-20)
Hint: Jujutsu uses libssh2, which doesn't respect ~/.ssh/config. Does `ssh -F /dev/null` to the host work?
error: process didn't exit successfully: `target\release\jj.exe git push` (exit code: 1)

[HybridEidolon]

Seeing as how we'll be moving to using git itself to handle pushes and fetches and the support for that has landed in 0.26.0, I'm gonna close this.