Skip to content

Macos ci tweaks#2584

Merged
kinnison merged 4 commits intorust-lang:masterfrom
kinnison:macos-ci-tweaks
Nov 29, 2020
Merged

Macos ci tweaks#2584
kinnison merged 4 commits intorust-lang:masterfrom
kinnison:macos-ci-tweaks

Conversation

@kinnison
Copy link
Copy Markdown
Contributor

This may fix #2583 if we're lucky

@kinnison kinnison added this to the 1.23.1 milestone Nov 29, 2020
@kinnison
Copy link
Copy Markdown
Contributor Author

@jeroen how do you feel about this?

@kinnison
Copy link
Copy Markdown
Contributor Author

I think this is good, the otool output suggests it's not dynlinking liblzma - @shepmaster do you think this is okay? If so, I'll try and prep/stage a 1.23.1 ready for @pietroalbini or another t-release member to do a release tomorrow.

@jeroen
Copy link
Copy Markdown

jeroen commented Nov 29, 2020

I don't understand how this changes anything, LZMA_API_STATIC is a preprocessor macro that is only relevant on Windows, I don't think it changes the linking behavior?

@kinnison
Copy link
Copy Markdown
Contributor Author

I don't understand how this changes anything, LZMA_API_STATIC is a preprocessor macro that is only relevant on Windows, I don't think it changes the linking behavior?

It's an environment variable which controls how lzma-sys links the library. In static mode it will not dynamically link to it.

The otool output in the completed actions run here suggest it has succeeded in that.

@jeroen
Copy link
Copy Markdown

jeroen commented Nov 29, 2020

Ah okay I wasn't aware that lzma-sys uses the same name as the C macro.

I'm not 100% confident the otool diagnostic is correct, did you confirm that the dylib shows up before the fix? Also maybe you should explicitly fail if there are links to /usr/local/ to prevent the same problem from appearing in other libs. The same may happen with for example libcurl.

GHA images have hundreds of 3rd party homebrew libs preinstalled and they get updated for each release so if you don't want to depend on those, I think the safest method is to get them out of the way.

On my GHA jobs that build deployed binaries, I always first uninstall homebrew, or the cheaper version:

rm -f /usr/local/lib/*.dylib

So then you can be confident you're not accidentally linking to locally installed 3rd party libs.

@kinnison
Copy link
Copy Markdown
Contributor Author

I did not check the otool output before the fix, but that's easy enough for me to try. I only worry that clearing /usr/local/lib might break other things, but I'm not a macos expert.

@shepmaster
Copy link
Copy Markdown
Member

The changes in general seem reasonable. I don't suppose there's a way of getting that artifact so that @jeroen can test it locally?

@kinnison
Copy link
Copy Markdown
Contributor Author

kinnison commented Nov 29, 2020

I've not worked out how to exfiltrate artifacts from gha builds without yet another s3 bucket somewhere.

@kinnison
Copy link
Copy Markdown
Contributor Author

Okay, so I can confirm that without the fix, otool shows /usr/local stuff present in the link. I'll see if I can make that cause a failure next.

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
@kinnison kinnison force-pushed the macos-ci-tweaks branch 2 times, most recently from 0fd4bfe to 5c3b0d1 Compare November 29, 2020 16:08
@kinnison
Copy link
Copy Markdown
Contributor Author

Yep, that worked. I'm now trying a run where we save the artifacts, if that works then @Jeoren can try the new macos build?

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
@kinnison
Copy link
Copy Markdown
Contributor Author

@jeroen Any chance you can test https://github.com/rust-lang/rustup/suites/1578429346/artifacts/28537948 ?

@jeroen
Copy link
Copy Markdown

jeroen commented Nov 29, 2020

Thanks! I've tested this on VM's with MacOS 10.11 and 10.13 and both work.

@kinnison
Copy link
Copy Markdown
Contributor Author

@jeroen Fantastic, thank you.

@kinnison kinnison merged commit 5392827 into rust-lang:master Nov 29, 2020
@kinnison kinnison deleted the macos-ci-tweaks branch November 29, 2020 20:47
@Jeoren
Copy link
Copy Markdown

Jeoren commented Nov 30, 2020 via email

@kinnison
Copy link
Copy Markdown
Contributor Author

@Jeoren Sorry, that was a typo on my part, I apologise for the spam.

@Eric-Arellano
Copy link
Copy Markdown

Eric-Arellano commented Dec 1, 2020

Hey @kinnison, thank you for promptly fixing this!

It sounds like you're planning a 1.23.1 release soon. That would help us a lot over at https://github.com/pantsbuild/pants for how we install Rust in our CI.

Thanks for all you do for the Rust ecosystem! 🦀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rustup not working on MacOS 10.13

5 participants