Linux dist: don't include unique symbols in libLLVM

[jethrogb]

Fixes #76980

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[jethrogb]

Can I get a try build?

Do we need to do something similar on other Linux targets? We don't build libstdc++ for every target today, just using the system-supplied one. Building a custom version seems like a lot of effort and maintenance headache going forward, if we need a multi-platform solution I'd advocate for using LLVM's libcxx or option #2.

[Mark-Simulacrum]

AFAIK only the x86_64-unknown-linux-gnu target on master today has LLVM built as a dylib (everything else statically links it). I believe we determined that a static link doesn't have this problem so we don't currently need to do this elsewhere?

[Mark-Simulacrum]

@bors try

[bors]

⌛ Trying commit d2ad472 with merge 7190d347441d18c5139f2301983961b062af5aa1...

[jethrogb]

For testing, need to run nm or objdump on the final artifacts. What we be a good place to insert such a test?

[Mark-Simulacrum]

Hm, it's hard -- we don't run tests on dist builders usually. I guess we could start a new testsuite for dist builders.

cc @pietroalbini on dist tests

[jethrogb]

AFAIK only the x86_64-unknown-linux-gnu target on master today has LLVM built as a dylib (everything else statically links it).

I think the llvm.link_shared config.toml option controls this, but I can't find that it's set to true anywhere for the Linux dist builder?

[Mark-Simulacrum]

ThinLTO, set https://github.com/rust-lang/rust/blob/master/src/ci/docker/host-x86_64/dist-x86_64-linux/Dockerfile#L98, implies shared linking currently.

[bors]

☀️ Try build successful - checks-actions
Build commit: 7190d347441d18c5139f2301983961b062af5aa1 (7190d347441d18c5139f2301983961b062af5aa1)

[jethrogb]

Confirmed that the try build doesn't reproduce https://github.com/Mark-Simulacrum/rust-issue-76980

[Mark-Simulacrum]

Ok, sounds good. I'm going to go ahead and r+ this -- I think it's the right fix and I want to get nightly fixed -- we can figure out how to test it in a follow-up PR. Thinking a bit more, I think we can enable ThinLTO on one of the test builders and just add the nm commands as a run-make test -- does that sound plausible to you @jethrogb?

@bors r+

[bors]

📌 Commit d2ad472 has been approved by Mark-Simulacrum

[Mark-Simulacrum]

@bors rollup=never p=1

[jethrogb]

I think we can enable ThinLTO on one of the test builders and just add the nm commands as a run-make test -- does that sound plausible to you @jethrogb?

I don't think so because I assume the test builders don't use our custom libstdc++? And I also want to guard against future changes in e.g. the Dockerfile/GCC upgrades messing up the dist build accidentally.

[Mark-Simulacrum]

Right, I was saying that we would try to match the configuration on linux dist builder on a test builder.

[sami2020pro]

0k

…

On Wed, 11 Nov 2020, 21:13 Mark Rousskov, ***@***.***> wrote: Right, I was saying that we would try to match the configuration on linux dist builder on a test builder. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#78946 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APOQON44LVDHBE3FLYDMLCLSPLELRANCNFSM4TSAKKPA> .

[jethrogb]

match the configuration on linux dist builder on a test builder.

Oh I see. I think that's a little more involved than just "enabling thinlto". We can try that, basically reusing the Dockerfile? I have a couple of concerns:

• The dist builder uses a super old version of Debian. Would we be able to run the whole test suite?
• The dist builder currently builds GCC and Clang prior to running x.py, that takes about an hour. Do we have room in our time budget for that?

[Mark-Simulacrum]

GCC and clang builds are docker-cached, so they don't really affect our time budget that much. We bounce anyway when those get invalidated I believe.

It's a good question -- I'm not sure if we can run the whole test suite. Maybe a starting goal is to see if we can add e.g. x.py test src/tools/run-make to the current linux dist builder and leave it at that.

[bors]

⌛ Testing commit d2ad472 with merge 5404efc...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing 5404efc to master...