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

build failure of openssl-sys on illumos within rustup project #1839

Closed
rbtcollins opened this issue Mar 12, 2023 · 9 comments
Closed

build failure of openssl-sys on illumos within rustup project #1839

rbtcollins opened this issue Mar 12, 2023 · 9 comments
Labels

Comments

@rbtcollins
Copy link

For more context - rust-lang/rustup#3263

In https://github.com/rust-lang/rustup/actions/runs/4398020553/jobs/7701559253 (which will get deleted at some point) we see

install libcrypto.a -> /checkout/target/x86_64-unknown-illumos/release/build/openssl-sys-6ea040a0afa3c9d0/out/openssl-build/install/lib/libcrypto.a
  Makefile:320: recipe for target 'install_dev' failed

  --- stderr
  crypto/init.c: In function 'OPENSSL_atexit':
  crypto/init.c:763:11: warning: variable 'handlersym' set but not used [-Wunused-but-set-variable]
           } handlersym;
             ^~~~~~~~~~
  x86_64-illumos-ar: creating apps/libapps.a
  /bin/sh: 1: granlib: not found
  x86_64-illumos-ar: creating libssl.a
  /bin/sh: 1: granlib: not found
  x86_64-illumos-ar: creating libcrypto.a
  /bin/sh: 1: granlib: not found
  /bin/sh: 5: granlib: not found
  make: *** [install_dev] Error 127
  thread 'main' panicked at '


  Error installing OpenSSL:
      Command: cd "/checkout/target/x86_64-unknown-illumos/release/build/openssl-sys-6ea040a0afa3c9d0/out/openssl-build/build/src" && "make" "install_dev"
      Exit status: exit status: 2


      ', /cargo/registry/src/jackfan.us.kg-1ecc6299db9ec823/openssl-src-111.25.1+1.1.1t/src/lib.rs:509:13
  stack backtrace:
     0: rust_begin_unwind
               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/std/src/panicking.rs:575:5
     1: core::panicking::panic_fmt
               at /rustc/2c8cc343237b8f7d5a3c3703e3a87f2eb2c54a74/library/core/src/panicking.rs:64:14
     2: openssl_src::Build::run_command
     3: openssl_src::Build::build
     4: build_script_main::find_vendored::get_openssl
     5: build_script_main::find_openssl
     6: build_script_main::main
     7: core::ops::function::FnOnce::call_once
  note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
warning: build failed, waiting for other jobs to finish...
Error: Process completed with exit code 101.

This used to build a couple months back but we've updated our openssl-sys version since and now get this.

@Skepfyr
Copy link
Collaborator

Skepfyr commented Mar 12, 2023

I'd bet on this being caused by alexcrichton/openssl-src-rs#173. Ultimately it appears to be caused by cc assuming that illumos has granlib installed and you don't appear to have that command available: (source)

                } else if target.contains("illumos") {
                    // The default 'ar' on illumos uses a non-standard flags,
                    // but the OS comes bundled with a GNU-compatible variant.
                    //
                    // Use the GNU-variant to match other Unix systems.
                    name = format!("g{}", tool);
                    self.cmd(&name)

@jonhoo
Copy link

jonhoo commented Mar 13, 2023

That logic came from @smklein over in rust-lang/cc-rs#585. If gar isn't the right tool to invoke, then the logic over in cc-rs should be updated (or an issue should be filed there):

https://github.com/rust-lang/cc-rs/blob/06c1289e35a3088b54d4c634c5bf327fb7ad36e4/src/lib.rs#L2841-L2847

As a workaround you can also explicitly set the AR environment variable to override.

@smklein
Copy link

smklein commented Mar 13, 2023

I'm pretty sure gar and granlib are the right tools for us to be calling on illumos - as the comment says, the g- prefix ensures that illumos uses a version of the tool with arguments that are aligned with other systems.

It looks like these tools are bundled in gnu-binutils, which should be installable via pkg install gnu-binutils. Would it be possible to add that to whatever image is running the illumos job?

The (IMO more complex) alternative would be to pick through all the flags for illumos, and pass them differently - the non-g--prefixed ar, as, ld, ranlib, etc all have slightly different calling conventions on illumos compared to other platforms.

@Rustin170506
Copy link

I'm pretty sure gar and granlib are the right tools for us to be calling on illumos - as the comment says, the g- prefix ensures that illumos uses a version of the tool with arguments that are aligned with other systems.

It looks like these tools are bundled in gnu-binutils, which should be installable via pkg install gnu-binutils. Would it be possible to add that to whatever image is running the illumos job?

In rustup, we fetch the docker from rust CI cache. The dockerfile for illumos job from https://github.com/rust-lang/rust/blob/master/src/ci/docker/host-x86_64/dist-x86_64-illumos/Dockerfile.

We use it in https://github.com/rust-lang/rustup/blob/master/ci/docker/x86_64-unknown-illumos/Dockerfile

FROM rust-x86_64-unknown-illumos

ENV \
    AR_x86_64_unknown_illumos=x86_64-illumos-ar \
    CC_x86_64_unknown_illumos=x86_64-illumos-gcc \
    CXX_x86_64_unknown_illumos=x86_64-illumos-g++ \
    CARGO_TARGET_X86_64_UNKNOWN_ILLUMOS_LINKER=x86_64-illumos-gcc

It seems we already set the AR environment. But it didn't work.

@jonhoo
Copy link

jonhoo commented Mar 14, 2023

Oh now that's interesting. The way that's supposed to get picked up is:

  1. get_base_archiver_variant calls env_tool
  2. env_tool calls get_var
  3. get_var checks AR_{target}

Clearly something in that path doesn't work, but it's hard to tell exactly what without access to an illumos system. Any chance you could try adding some debug prints to the above and run the CI to see where the envvar does/does not get picked up?

@jonhoo
Copy link

jonhoo commented Mar 14, 2023

I think we should also turn this into an issue on the cc-rs repo.

@Rustin170506
Copy link

I think we should also turn this into an issue on the cc-rs repo.

Opened rust-lang/cc-rs#798

@Rustin170506
Copy link

Rustin170506 commented Mar 15, 2023

Oh now that's interesting. The way that's supposed to get picked up is:

  1. get_base_archiver_variant calls env_tool
  2. env_tool calls get_var
  3. get_var checks AR_{target}

Clearly something in that path doesn't work, but it's hard to tell exactly what without access to an illumos system. Any chance you could try adding some debug prints to the above and run the CI to see where the envvar does/does not get picked up?

I checked the log, and actually we missing granlib in the docker image. So I fixed this issue by add this env:

ENV \
  AR_x86_64_unknown_illumos="x86_64-illumos-ar" \
  RANLIB_x86_64_unknown_illumos="x86_64-illumos-ranlib" \
  CC_x86_64_unknown_illumos=x86_64-illumos-gcc \
  CXX_x86_64_unknown_illumos=x86_64-illumos-g++ \
  CARGO_TARGET_X86_64_UNKNOWN_ILLUMOS_LINKER=x86_64-illumos-gcc

Thanks for your help!

@Skepfyr
Copy link
Collaborator

Skepfyr commented Mar 15, 2023

Sounds like this has been fixed. Thanks for raising as I think this is the root cause of a couple of other issues raised in the last week.

@Skepfyr Skepfyr closed this as completed Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants