Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

redis: only link libatomic on i386 #15092

Merged
merged 1 commit into from Jun 9, 2022
Merged

redis: only link libatomic on i386 #15092

merged 1 commit into from Jun 9, 2022

Conversation

dgilman
Copy link
Contributor

@dgilman dgilman commented Jun 8, 2022

Closes: https://trac.macports.org/ticket/65311

Description

This will at least stop the bleeding but should hopefully resurrect builds on i386 too.

Type(s)
  • bugfix
  • enhancement
  • security fix
Tested on

macOS 10.15.7 19H1922 x86_64
Xcode 12.4 12D4e

Verification

Have you

  • followed our Commit Message Guidelines?
  • squashed and minimized your commits?
  • checked that there aren't other open pull requests for the same change?
  • referenced existing tickets on Trac with full URL?
  • checked your Portfile with port lint --nitpick?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@@ -54,7 +54,7 @@ configure.cppflags-replace \
-isystem${prefix}/include

# https://trac.macports.org/ticket/58712
if {${os.major} <= 18} {
if {${build_arch} eq "i386"} {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will not be correct for universal builds. Use the facilities of the muniversal portgroup to add the -latomic flag only for the i386 part of universal builds.

Copy link
Member

@mascguy mascguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the outstanding comment from @ryandesign re: Universal builds, this fixes the build issues for earlier macOS releases. (Haven't tested across-the-board, but can confirm for 10.12 through 10.14.)

@kencu
Copy link
Contributor

kencu commented Jun 9, 2022

clang doesn’t use libatomic. It’s a gcc-only library.

clang on i386 uses builtins in compiler_rt. On x86_64 it uses processor intrinsics.

@kencu
Copy link
Contributor

kencu commented Jun 9, 2022

ie this whole fix, adding libatomic to fix the i386 build with clang, is not the right fix.

@dgilman
Copy link
Contributor Author

dgilman commented Jun 9, 2022

I don't have the hardware to test any of this stuff out so I've been flying blind. I will back out the change then. The redis makefile jumps through a bunch of hoops and needs a bunch of environment variable hand holding so I suspect that even with the hardware to test it on this is going to be an involved change.

@mascguy
Copy link
Member

mascguy commented Jun 9, 2022

I don't have the hardware to test any of this stuff out so I've been flying blind. I will back out the change then. The redis makefile jumps through a bunch of hoops and needs a bunch of environment variable hand holding so I suspect that even with the hardware to test it on this is going to be an involved change.

@barracuda156 Sergey, would you be interested in helping to test a Universal build for this port?

@mascguy
Copy link
Member

mascguy commented Jun 9, 2022

Given that users need this fix ASAP, let's merge.

@barracuda156 Sergey, we can fix Universal via a subsequent PR if necessary, depending on your results. Let us know, and we'll go from there!

@mascguy mascguy merged commit 96579f7 into macports:master Jun 9, 2022
@barracuda156
Copy link
Contributor

Given that users need this fix ASAP, let's merge.

@barracuda156 Sergey, we can fix Universal via a subsequent PR if necessary, depending on your results. Let us know, and we'll go from there!

@mascguy Sure, I will save this PR link and test +universal on PPC machines, but please allow me to postpone that until the end of month! I am on a trip, and have no access to my PPC hardware at the moment. Should be back by July 1.

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