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

exact-image: Use the right -arch flags and other flags #3385

Merged
merged 8 commits into from
Jan 15, 2019

Conversation

ryandesign
Copy link
Contributor

Description

This PR began in an effort to fix the universal build failure I reported here. But there are so many things wrong in this build system that it took a larger effort to fix. I'm submitting it as a PR so that I can separate each logical change into its own commit, and to get a second opinion on whether I've done anything wrong. But it builds universal now for me, where it did not before. @MarcusCalhoun-Lopez, maybe you can take a look, since I'm changing a lot of stuff you added in the update to 1.0.1.

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

macOS 10.13.6 17G65
Xcode 9.4.1 9F2000

Verification

Have you

  • checked your Portfile with port lint?
  • tried existing tests with sudo port test?
  • tried a full install with sudo port -vst install?
  • tested basic functionality of all binary files?

@macportsbot macportsbot added by: member Created by a member with commit rights maintainer: none labels Jan 9, 2019
@macportsbot
Copy link

Travis Build #4841 Failed.

Lint results
--->  Verifying Portfile for exact-image
--->  0 errors and 0 warnings found.

Port exact-image fail on xcode9.4. Log
Port exact-image fail on xcode8.3. Log
Port exact-image fail on xcode7.3. Log
Port exact-image fail on xcode10.1. Log

@ryandesign
Copy link
Contributor Author

checking whether the C++ compiler works ... no
A C++ compiler is not installed or does not work correctly.
A C++ compiler is vital for exact-image - so you need to install it first.

That's not what it did on my system. Hmm...

@ryandesign
Copy link
Contributor Author

Ah, I have the bash @4.4.23 port installed. Looks like some syntax I used doesn't work with macOS stock bash version 3.

Use CPPFLAGS and CFLAGS/CXXFLAGS for most configure tests that involve
the compiler.

Use only CPPFLAGS when checking headers. Using CFLAGS/CXXFLAGS as well
would cause a configure failure when using the universal variant.

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

Instead of checking $1 for an exact string match of "cc" or "c++", check
only the first word of $1, because in some use cases $1 also contains
some flags.
Needed to use the right -arch flags, which is needed to build universal.

Because this also now causes MacPorts optimization flags and C++ stdlib
flags to be used, increase the revision to rebuild.

See: https://trac.macports.org/ticket/57607
@macportsbot
Copy link

Travis Build #4846 Passed.

Lint results
--->  Verifying Portfile for exact-image
--->  0 errors and 0 warnings found.

Port exact-image success on xcode9.4. Log
Port exact-image success on xcode8.3. Log
Port exact-image success on xcode7.3. Log
Port exact-image success on xcode10.1. Log

@ryandesign
Copy link
Contributor Author

Ok, this version of my modifications seems to work with bash 2, 3, or 4.

@pmetzger
Copy link
Member

@ryandesign Will you be merging this on your own?

@ryandesign
Copy link
Contributor Author

Yes, unless someone reviews it and finds something wrong with it.

Copy link
Contributor

@MarcusCalhoun-Lopez MarcusCalhoun-Lopez left a comment

Choose a reason for hiding this comment

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

Everything seems to build correctly for me.
A minor issue is that patch-save-config.log.diff does not seem to apply cleanly.

@ryandesign ryandesign merged commit 8205f1d into macports:master Jan 15, 2019
@ryandesign ryandesign deleted the exact-image-fixes branch January 15, 2019 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
by: member Created by a member with commit rights maintainer: none
Development

Successfully merging this pull request may close these issues.

4 participants