1635036 - build/moz.configure/old.configure uses shell to run autoconf, breaking when wrapper programs are used

Status: NEW

(Firefox Build System :: General, enhancement, P5)

Description

[Michael Stapelberg]

Attachment: Patch to no longer use an explicit shell process

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/80.0.3987.132 Safari/537.36

Steps to reproduce:

(I encountered this issue when building mozjs 60.9.0esr, but checked HEAD at https://dxr.mozilla.org/mozilla-central/source/build/moz.configure/old.configure#132 and it’s still present.)

On my system, all programs are wrapped by a small program that sets relevant environment variables to ensure that same dependencies are used as were present at build time (“hermetic packages”, I have a blog post upcoming in case anyone is interested).

This generally works fine, but I encountered an issue when building mozjs (for polkit): build/moz.configure/old.configure assumes that autoconf is a shell script (it isn’t on my machine) and runs [shell, autoconf], which fails.

Actual results:

The firefox build system tried to run autoconf via the shell.

Expected results:

autoconf should have been run directly, without an explicit shell in between.

Comment 1

[Nathan Froyd [:froydnj]]

Shouldn't your wrapper program have been run as if wrapper shell autoconf [autoconf-args...] and done the right thing?

Comment 2

[Michael Stapelberg]

See https://website-review.zekjur.net/pull/hermetic/posts/2020-05-04-distri-hermetic-packages/ for the (work-in-progress) blog post.

Shouldn't your wrapper program have been run as if wrapper shell autoconf [autoconf-args...] and done the right thing?

Not 100% sure I follow, so let me outline a few things more clearly and hope that this either answers your question or lets you clarify it:

The wrapper in this case is /ro/autoconf-amd64-2.69-6/bin/autoconf, which is a compiled program. This wrapper will essentially execve("/ro/autoconf-amd64-2.69-6/out/bin/autoconf", …) (note the /out/ path component), which is the autoconf shell script.

The Firefox build system runs [shell, wrapper], e.g. /bin/bash /ro/autoconf-amd64-2.69-6/bin/autoconf. This breaks because the shell doesn’t support running compiled programs.

Does that make sense? Thanks.

Comment 3

[Nathan Froyd [:froydnj]]

Ah, that makes more sense, thanks for the explanation.

The patch is not correct in general, because autoconf cannot be directly invoked on Windows via subprocess.check_output.

Even if the patch handled that case, I think general policy would indicate that the burden is on unusual build environments to provide local patches to ensure that packages build correctly in said environment.

Comment 4

[Michael Stapelberg]

The patch is not correct in general, because autoconf cannot be directly invoked on Windows via subprocess.check_output.

Ah, thanks for providing the background.

Could we enable this behavior conditionally only for Windows?

If one needs to rely on implementation details of autoconf to run it on Windows, so be it, but on Linux, this is unnecessary, bad style, and breaks my build :)

Comment 5

[Nathan Froyd [:froydnj]]

(In reply to Michael Stapelberg from comment #4)

Could we enable this behavior conditionally only for Windows?

We could, but as I said in comment 3, we would typically put the burden on unusual build environments to provide local patches to ensure that packages build correctly in said environment.