Remove redundant -Wl,-syslibroot

[madsmtm]

Since -isysroot is set, Clang already passes this when invoking the linker.

See #56833 for when the -isysroot was originally added, but didn't remove the unnecessary linker flag.

CC @BlackHoleFox
r? shepmaster

[BlackHoleFox]

Does this work with every linker or just Apple/clang's default one? Curious if linkers such as mold handle this Apple-ism correctly.

[madsmtm]

Does this work with every linker or just Apple/clang's default one? Curious if linkers such as mold handle this Apple-ism correctly.

This only changes the behaviour used when a compiler driver like Clang or GCC is used to link, when we invoke the linker directly the -syslibroot flag is still passed as usual.

I know that it works for GCC, where this behaviour is explicitly documented, I'm not really familiar with other compiler drivers with iOS support?

[BlackHoleFox]

oh I see. Missed that it was just compiler drivers (guessing that is the Cc::Yes enum variant being matched), so yeah this seems like it'd be fine. I also don't know anything besides clang and gcc that can build Apple targets.

[madsmtm]

Same as in #121430, unsure who has macOS/Xcode experience that I could assign this to.

r? compiler

[cjgillot]

r? compiler

[Nadrieril]

same 😅
r? compiler

[thomcc]

This looks right and is worth landing.

[thomcc]

r=me if someone from t-compiler looks over and decides it's fine for me to r=me on this (despite it not being a libs thing).

[pnkfelix]

@thomcc Out of curiosity, what is the benefit of removing the redundancy here? Just that it simplifies our generated command line?

[pnkfelix]

I don't object to this; it sounds like the most obvious risks are implicitly addressed (e.g. "does it work if one changes to link with mold", as discussed in comments above).

So I'm going to approve it, though its also not yet clear to me why its important to make this change...

[pnkfelix]

@bors r+

[bors]

📌 Commit f1548ec has been approved by pnkfelix

It is now in the queue for this repository.

[bors]

⌛ Testing commit f1548ec with merge 22a2425...

[bors]

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 22a2425 to master...

[rust-timer]

Finished benchmarking commit (22a2425): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

This benchmark run did not return any relevant results for this metric.

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.0%	[1.9%, 2.1%]	2
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.2%	[-2.3%, -2.2%]	2
Improvements ✅ (secondary)	-2.9%	[-3.5%, -2.3%]	14
All ❌✅ (primary)	-0.1%	[-2.3%, 2.1%]	4

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 677.195s -> 676.361s (-0.12%)
Artifact size: 316.01 MiB -> 316.01 MiB (-0.00%)

[madsmtm]

why its important to make this change

There was no other motivation than me being confused about it, exactly because it's redundant.