mrustc icon indicating copy to clipboard operation
mrustc copied to clipboard

Published 20 hours ago • verified publisher icon thepowersgang

Fix bootstrapping for macOS for 1.39.0

Open arlosi opened this issue 2 years ago • 11 comments

  • Modify 1.39.0 patch to remove assertions about structure size due to alignment differences and workaround a linker issue searching for CFMutableAttributedStringGetTypeID by pulling in patch from core-foundation
  • Fix incorrect signature in LLVM add_overflow intrinsics
  • Change the macOS target name to x86_64-apple-darwin to match Rust triple.

cc @woachk

arlosi avatar Oct 18 '21 16:10 arlosi

Hello,

Please copy the RUSTC_TARGET define for Darwin from minicargo.mk, to take into account arm64 Mac hardware. (In run_rustc/Makefile)

The build-1.39.0.sh change also doesn’t need to be there.

Apart from those things, this looks good.

woachk avatar Oct 19 '21 17:10 woachk

Just the peanut gallery here, but can you explain why the static_assert_size calls have been removed?

Edit: Sorry, I misread the PR. Coffee still kicking in...

evanmiller avatar Dec 15 '21 12:12 evanmiller

Taking a serious look at this now (sorry it's taken so long).

  • Could you update the patch file to make the size checks conditional on not using mrustc? not(rust_compiler = "mrustc")
  • I'm still kinda curious as to why there was a null pointer in that deref chain
  • Is -darwin always the correct target name? Or is-macos also valid (if it is also valid, then don't remove the target from the spec)

thepowersgang avatar Dec 27 '21 11:12 thepowersgang

@arlosi Sorry, but a large amount of your changes have snuck in via other means. Could you rebase this with just the remaining changes (and address the comments in https://github.com/thepowersgang/mrustc/pull/192#issuecomment-1001519202)

thepowersgang avatar Jan 05 '22 09:01 thepowersgang

Sure, I can update the PR today.

  • I'll add the conditional compilation for not(rust_compiler = "mrustc")
  • Looks like the null deref question is resolved now.
  • -darwin is always the correct name AFAIK. -macos isn't mentioned in the official rust compiler documentation.

arlosi avatar Jan 05 '22 17:01 arlosi

I don't have access to the Mac hardware I used when I originally made this change, so I haven't re-validated the full bootstrap of mrustc -> 1.39.0 -> 1.39.0 -> 1.40.0 with the latest PR updates. It worked with the original version of the change.

I have used a Mac CI machine to test that mrustc can build rustc 1.39.0 with this updated change but I can't test further than that.

arlosi avatar Jan 06 '22 16:01 arlosi

I believe that most of these changes were included in #241 - which has now been merged.

thepowersgang avatar Feb 07 '22 12:02 thepowersgang

@thepowersgang if you willing to merge it, you may use version which is applied to last master and which I've used for my MacPorts port: https://github.com/catap/mrustc/commit/3d24d451b23104b58f8afc9d07d1cd341b296f5a

catap avatar Feb 07 '22 22:02 catap

Wait, this is for 1.39's patch set, not 1.54's. @arlosi Part of this has been applied, and some has been addressed. Could you update the patch set?

thepowersgang avatar Feb 07 '22 23:02 thepowersgang

@thepowersgang my commit is for 1.39.0 also ;)

I can't move forward with 1.54.0.

catap avatar Feb 07 '22 23:02 catap

Oh yeah, point still stands. This PR needs to be either updated against master, or closed.

thepowersgang avatar Feb 08 '22 00:02 thepowersgang

Most of this change has already been merged via other PRs, and work has moved to 1.54. Closing.

arlosi avatar Jan 06 '23 22:01 arlosi