[build] Don't reset 'swiftlib_link_flags_all' unnecessarily

[finagolfin]

Done in #29017 but this variable has already been set just above, so all subsequent flags added are wiped, which breaks setting the soname for target libraries on Android (@SDGGiesbrecht, let me know if this patch fixes the Android linking issue you mentioned on the forum, it should), among others.

@devincoughlin, I don't know if this was a temporary change you needed or if those prior modified flags were affecting your Catalyst work somehow, let me know.

[SDGGiesbrecht]

let me know if this patch fixes the Android linking issue you mentioned

Quite possibly, but I won’t be able to definitively test it until it is merged and Azure has created an artifact with it. I left a reminder for myself to check in a few days.

[drodriguez]

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - b0498fa03d6baeca46b956237c5edd427315204b

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - b0498fa03d6baeca46b956237c5edd427315204b

[finagolfin]

Both CI failures appear to be spurious: the mac one didn't even get past git checkout of other Swift repositories, while I have no idea why that SwiftPM test would fail on linux, as it passed for me locally on an Android host with this pull.

[drodriguez]

@swift-ci please test

[finagolfin]

Good to go?

[drodriguez]

I think @devincoughlin should pitch in before merging this. The line might be internally important for Apple, but it is obviously affecting some other platforms. In the meantime, can you adapt this after #29807?

[finagolfin]

Rebased.

[drodriguez]

@swift-ci please test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - b0498fa03d6baeca46b956237c5edd427315204b

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - b0498fa03d6baeca46b956237c5edd427315204b

[finagolfin]

@devincoughlin, this broke the Android SDK a couple months ago, can you chime in?

[finagolfin]

I think we can go ahead with this, it's fairly obvious that he made a mistake.

[finagolfin]

Ping @CodaFi, the Swift SDK for Android has been broken for the last couple months since this line slipped in, would be good to get this in before the 5.3 branch.

[CodaFi]

I am not the right person to review CMake changes.

Saleem added this line, so @compnerd could you look this over?

[finagolfin]

Saleem added this line

No, that's what a quick git blame will show, but it was actually added by the Catalyst changes, see the OP. Saleem simply moved it in one of his subsequent CMake refactors. I don't care who reviews it, but @drodriguez seemed okay with it, and it has sat in limbo for several weeks since the original author has not responded to repeated pings.

@CodaFi, I was just hoping you'd prod someone about this, or it's a really simple one-line removal to review, which doesn't really make sense before this patch, as explained in the OP.

[CodaFi]

Ah, then @devincoughlin is probably right.

[compnerd]

@CodaFi I think that this is correct, but it should be tested with the catalyst build as it was previously dropping the linker flags.

[compnerd]

Given that it is dropping flags, let’s merge cause the flags should be applied, we can filter our flags that catalyst may end up with issues on.

[SDGGiesbrecht]

@buttaface, the issue I noted is no longer a problem (though so much time passed in between that it’s hard to say whether this was the thing that fixed it).

[finagolfin]

Well, it should be pretty easy, just check what commit of Swift master worked, because this pull was just reverted. If the few commits with this patch work, but the latest master doesn't, you know that this fixed it (or maybe the builds you're using have this and other fixes locally applied, I don't know what they do).