1643099 - Since the 0.26.0 release geckodriver for Linux is no longer statically linked

Status: RESOLVED FIXED

(Testing :: geckodriver, defect, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Originally filed as: https://github.com/mozilla/geckodriver/issues/1679

Since version 0.26.0 the geckodriver binary is no longer statically linked:

$ tar xvf geckodriver-v0.25.0-linux64.tar.gz 
geckodriver
$ ldd geckodriver
	not a dynamic executable
henrik@henrik-VirtualBox:~/Downloads$ rm geckodriver
henrik@henrik-VirtualBox:~/Downloads$ tar xvf geckodriver-v0.26.0-linux64.tar.gz 
geckodriver
henrik@henrik-VirtualBox:~/Downloads$ ldd geckodriver
	linux-vdso.so.1 (0x00007ffe07db4000)
	libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fd9cb525000)
	librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fd9cb31d000)
	libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fd9cb0fe000)
	libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fd9caee6000)
	libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd9caaf5000)
	/lib64/ld-linux-x86-64.so.2 (0x00007fd9cbd3a000)
	libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fd9ca757000)

The same is still the case for geckodriver binaries as produced by the toolchain jobs:

https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&searchStr=geckodriver

So this most likely regressed by bug 1534533 given that we used the archives from the toolchain builds for the release. Looks like to be similar to bug 1584530 where we removed the msvcrt dependency on Windows, and as such adding the following line should fix it:

https://searchfox.org/mozilla-central/rev/7cadba1d8b8feaec4615f5bb98aac4b8a719793c/taskcluster/scripts/misc/build-geckodriver.sh#36

export RUSTFLAGS="-Ctarget-feature=+crt-static"

By that I wonder if we would need something similar for Mac. Maybe even only for safety.

Comment 1

[Nick Alexander :nalexander [he/him]]

Looks like we need to target musl: see https://doc.rust-lang.org/1.9.0/book/advanced-linking.html#static-linking.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Yes, and that is what we used when building geckodriver via TravisCI:

https://github.com/mozilla/geckodriver/blob/release/.travis.yml#L18

    - os: linux
      env:
        - TARGET=x86_64-unknown-linux-musl
        - NAME=linux64
        - EXT=tar.gz
      dist: trusty
      sudo: required
      addons:
        apt:
          packages:
          - musl-tools
          - libbz2-dev

So we also need to install those two extra packages via apt, in case these aren't present yet.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So the musl target isn't installed. Does it mean we can add it to the existent linux64-rust toolchain, or shall we create a new one for geckodriver?

Comment 4

[Nathan Froyd [:froydnj]]

I vote to create a new one for geckodriver's use.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I'm trying this now but I hit the situation that rust-analysis is not available for i686-unknown-linux-musl, and it's marked as required in repack_rust.py. Would it be possible to mark rust-analysis optional, or do we really require it?

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I tried to add a new linux64-rust-cross-static toolchain build and it run just fine. But dependent toolchain builds cannot find the std and core libs:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5280fd7d191b0771c5cbea247e12156154ef956d&selectedTaskRun=IUA760YURKyA2BsOx7ngRA-0

Any idea what's wrong here?

Comment 7

[Nathan Froyd [:froydnj]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #6)

I tried to add a new linux64-rust-cross-static toolchain build and it run just fine. But dependent toolchain builds cannot find the std and core libs:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5280fd7d191b0771c5cbea247e12156154ef956d&selectedTaskRun=IUA760YURKyA2BsOx7ngRA-0

Any idea what's wrong here?

There are a couple problems afoot:

1. Your linux geckodriver builds specify a rust repack (linux64-rust-cross-static-1.43) that doesn't include the target x86_64-unknown-linux-gnu, so compiling things for the host (which is x86_64-unknown-linux-gnu) won't work.
2. You changed the mac geckodriver builds to use that same rust repack, which seems unnecessary and also causes the same problem as (1).
3. You changed the windows geckodriver builds to use that same rust repack, which has all the same problems as (1). You also changed the target that geckodriver is being compiled for, which doesn't seem necessary (and in fact might be actively harmful).

You should just make a separate toolchain that additionally contains the x86_64-unknown-linux-musl target, make the linux geckodriver builds consume that, make the linux geckodriver builds target x86_64-unknown-linux-musl, and leave everything else alone.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Nathan Froyd [:froydnj] from comment #7)

2. You changed the mac geckodriver builds to use that same rust repack, which seems unnecessary and also causes the same problem as (1).
3. You changed the windows geckodriver builds to use that same rust repack, which has all the same problems as (1). You also changed the target that geckodriver is being compiled for, which doesn't seem necessary (and in fact might be actively harmful).

I tried to match up what we used on github to build the binaries:
https://github.com/mozilla/geckodriver/blob/release/.travis.yml#L36

So for Windows we didn't use x86_64-pc-windows-msvc but x86_64-pc-windows-gnu. So with bug 1584530 we had to make the msvc builds to compile static too. Given that we don't need msvc why can't we just compile without it?

Comment 10

[Nathan Froyd [:froydnj]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #9)

(In reply to Nathan Froyd [:froydnj] from comment #7)

2. You changed the mac geckodriver builds to use that same rust repack, which seems unnecessary and also causes the same problem as (1).
3. You changed the windows geckodriver builds to use that same rust repack, which has all the same problems as (1). You also changed the target that geckodriver is being compiled for, which doesn't seem necessary (and in fact might be actively harmful).

I tried to match up what we used on github to build the binaries:
https://github.com/mozilla/geckodriver/blob/release/.travis.yml#L36

So for Windows we didn't use x86_64-pc-windows-msvc but x86_64-pc-windows-gnu. So with bug 1584530 we had to make the msvc builds to compile static too. Given that we don't need msvc why can't we just compile without it?

If you use x86_64-pc-windows-gnu on your CI and have used it for releases without people complaining, I guess that works. I was unaware of the history here.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Nathan Froyd [:froydnj] from comment #10)

If you use x86_64-pc-windows-gnu on your CI and have used it for releases without people complaining, I guess that works. I was unaware of the history here.

Given that we will need a geckodriver release soon, I'm dropping that for now. If it will cause trouble or need improvements we can do that change back to gnu. Otherwise lets continue to use the current toolchain.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Nathan, for linux32 I have a problem:

[task 2020-06-29T15:38:32.829Z] repack: rust-analysis marked unavailable for i686-unknown-linux-musl

It seems like we require that. Do you have an advice in how to handle that?

Comment 13

[Nick Alexander :nalexander [he/him]]

So for Windows we didn't use x86_64-pc-windows-msvc but x86_64-pc-windows-gnu. So with bug 1584530 we had to make the msvc builds to compile static too. Given that we don't need msvc why can't we just compile without it?

I can provide some context here: I'm quite certain that I moved to MSVC simply to follow suit with the large number of Rust binaries we build as toolchains already (sccache, minidump_stackwalk, others). All of those used MSVC. My dream here is to have a rust-toolchain kind or a is-rust: true flag that automatically does the cross-platform build configuration for the tasks, so that you get all targets built from Linux smoothly rather than having to monkey with each one and having them all slightly different. But maybe that's not reasonable with the MSVC/mingw split.

Comment 14

[Nathan Froyd [:froydnj]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #12)

Nathan, for linux32 I have a problem:

[task 2020-06-29T15:38:32.829Z] repack: rust-analysis marked unavailable for i686-unknown-linux-musl

It seems like we require that. Do you have an advice in how to handle that?

You could exclude i686-unknown-linux-musl from installing rust-analysis here:

https://searchfox.org/mozilla-central/source/taskcluster/scripts/misc/repack_rust.py#239-244

It's not particularly elegant, but I think since we're not using searchfox for these builds (?), we're not particularly worried about losing coverage here.

I think the longer-term fix would be to enable rust-analysis upstream in https://github.com/rust-lang/rust/blob/master/src/ci/docker/dist-i586-gnu-i586-i686-musl/Dockerfile by passing --enable-full-tools or similar to configure. I don't know how well-received that would be on the Rust side.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1643099 - [geckodriver] Build geckodriver as static executable for Linux.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f20f4cf01cd
[geckodriver] Build geckodriver as static executable for Linux. r=froydnj

Comment 17

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/7f20f4cf01cd