1844538 - build/cargo-linker should use os.system instead of os.execvp to handle apostrophe

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Petr Sumbera]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Firefox/102.0

Steps to reproduce:

I do have:

MOZ_CARGO_WRAP_LDFLAGS="-lpthread -Wl,-z,text '-Wl,-R,$ORIGIN'"

when cargo-linker was shell script it worked without problem.

After turning it into python script linker (gcc) is given '-Wl,-R,$ORIGIN' (with apostrohes) which it cannot handle and consider it as object file and tryies to load it.

This can be avoid by using os.system instead of os.execvp:

--- firefox-115.0.3/build/cargo-linker
+++ firefox-115.0.3/build/cargo-linker
@@ -54,5 +54,4 @@ args.append(f"-fsanitize={SANITIZERS[suf
             continue
     args.append(arg)
 
-wrap_ld = wrap_ld.split()
-os.execvp(wrap_ld[0], wrap_ld + args)
+os.system(wrap_ld + " " + ' '.join(args))

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Firefox Build System::General' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[BugBot [:suhaib / :marco]]

:glandium, since you are the author of the regressor, bug 1791476, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Mike Hommey [:glandium]]

Attachment: Bug 1844538 - Properly split environment variables in cargo-linker.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/087853f0aeda
Properly split environment variables in cargo-linker. r=firefox-build-system-reviewers,ahochheiden

Comment 5

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/087853f0aeda

Comment 6

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 1791476

Comment 7

[Dianna Smith [:diannaS]]

:glandium is this okay to ride the 117 trains? if so, please set 116 to wontfix. If not, just a reminder that RC builds tomorrow.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

I'm guessing we want this on ESR115 if Petr's reporting it?

Comment 9

[Mike Hommey [:glandium]]

I'm not sure, I feel like that -Wl,-R,$ORIGIN shouldn't even be required in the first place.

Comment 10

[Petr Sumbera]

(In reply to Mike Hommey [:glandium] from comment #9)

I'm not sure, I feel like that -Wl,-R,$ORIGIN shouldn't even be required in the first place.

Yes, it's not really required for Firefox itself. I just need this for our packaging system to not complain about library dependencies. Firefox libraries are installed into /usr/lib/firefox which is not default linker search path.

I can live with the patch file for version 115 time being. Thank you for fixing it!