Deduplicate native libs before they are passed to the linker

[ChrisDenton]

Stop spamming the linker with the same native library over and over again, if they directly follow from each other. This would help prevent this situation.

Issue #38460 has been open since 2016 so I think it's worth making an incomplete fix that at least addresses the most common symptom and without otherwise changing how Rust handles native libs. This PR is intended to be easy to revert (if necessary) when a more permanent fix is implemented.

[rust-highfive]

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

[joshtriplett]

This seems reasonable, but also, I think it's a good motivation to do #76992 as well. With that change, it'd be reasonable to deduplicate libraries even if they're not adjacent.

[petrochenkov]

@joshtriplett
This is a not a trivial problem, there are always issues like #84254 or https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/windows_gnu_base.rs#L33 that require preserving library order or disabling deduplication even in presence of --(start,end)-group (which also isn't necessarily supported on all platforms).

That's why I want to land the linking modifiers infra (#83507) first, and not change or fix any deduplication behavior (including #73319) until a modifier for disabling/enabling deduplication is added.
I plan to work on this in May/June (I just got busy with some unexpected life stuff last week, so I may not start this work immediately).

(FWIW, I don't think MSxDOS/ntapi#2 is purely a rustc's issue, with macro expansion you can do a lot of things slowing down you compilation deliberately, not necessarily related to linking.)

[ChrisDenton]

Consider the following linker library order:

libA mylib libA libA libA libB

As far as I'm aware (please correct me if I'm wrong), this is exactly the same as:

libA mylib libA libB

Which is all this PR currently does. However the following is not the same, even if the linker is allowed to circle back to the start:

libA mylib libB

In the first cases, if a needed symbol is unresolved in mylib it will first be looked for in libA. In the last case it will look for the unresolved symbols in libB. In essence, from the view of mylib, it's swapped the order of libA and libB:

libA mylib libB libA ...

Therefore I think this PR is the most that can be done without potentially breaking some linking strategies.

[petrochenkov]

Yeah, this PR should be pretty safe (from the description, I didn't check the code), modulo the "mingw linker + mixed import-static library" case linked above.

[jackh726]

I'm definitely not a good reviewer for this. So let's r? @petrochenkov

[ChrisDenton]

modulo the "mingw linker + mixed import-static library" case linked above.

In the specific libmsvcrt.a case this PR doesn't pose a problem due to the way it's linked. In the more general case, I would (naïvely?) hope that Mingw does not otherwise produce libraries that it's own linker can't properly consume. On the other hand, if it does then it would be good if there's a test case I can use.

Either way I guess one workaround would be to allow up to one repetition. Perhaps only if the target linker is gcc/windows. Though I am slightly reluctant to introduce too many temporary hacks.

[petrochenkov]

I would (naïvely?) hope that Mingw does not otherwise produce libraries that it's own linker can't properly consume

This is true, libmsvcrt.a is a very special case that can be solved by a linking modifier once linking modifiers (#83507) are implemented.

@bors r+ , this is the most conservative deduplication strategy, and once the modifiers are implemented we'll probably end up with aggressive deduplication (and opt-out when necessary) which is compatible with what this PR does.

[bors]

📌 Commit e40faef has been approved by petrochenkov

[Dylan-DPC-zz]

likely failure of rollup ?

@bors r-

[petrochenkov]

Seems unlikely.
@bors r+ rollup=iffy

[bors]

📌 Commit e40faef has been approved by petrochenkov

[bors]

⌛ Testing commit e40faef with merge b79abea265ac16320d751e3075208554fae99591...

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

  0  424M    0  926k    0     0   2261      0 54:38:56  0:06:59 54:31:57     0
  0  424M    0  926k    0     0   2256      0 54:46:12  0:07:00 54:39:12     0
  0  424M    0  926k    0     0   2250      0 54:54:58  0:07:01 54:47:57     0
  0  424M    0  926k    0     0   2248      0 54:57:54  0:07:02 54:50:52     0
curl: (56) OpenSSL SSL_read: Connection reset by peer, errno 54
clang+llvm-10.0.0-x86_64-apple-darwin/lib/libLLVMAnalysis.a: Lzma library error:  No progress is possible
tar: Error exit delayed from previous errors.
##[error]Process completed with exit code 1.
[command]/usr/local/bin/git version
git version 2.31.1
[command]/usr/local/bin/git config --local --name-only --get-regexp core\.sshCommand
[command]/usr/local/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :

[bors]

💔 Test failed - checks-actions

[Mark-Simulacrum]

@bors retry network error

[bors]

⌛ Testing commit e40faef with merge 2d11e25...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 2d11e25 to master...