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

Multiple OS X patches #9

Merged
merged 2 commits into from Jun 13, 2016

Conversation

Projects
None yet
2 participants
@FiloSottile
Contributor

FiloSottile commented May 20, 2016

With this, and PATH="/usr/local/opt/gnu-sed/libexec/gnubin:/usr/local/opt/coreutils/libexec/gnubin:$PATH" I was able to build for arm-linux-musleabihf and x86_64-linux-musl on El Capitan.

@FiloSottile FiloSottile changed the title from Patch elf.h circular dependency out of linux-4.4.10 to Multiple OS X patches May 20, 2016

FiloSottile added some commits May 20, 2016

Patch elf.h circular dependency out of linux-4.4.10
This enables building on platforms without a native elf.h, like OS X

Fixes #8
@FiloSottile

This comment has been minimized.

Contributor

FiloSottile commented Jun 2, 2016

Ping? :) I'm really keen on publishing the Homebrew Formula...

@richfelker

This comment has been minimized.

Owner

richfelker commented Jun 2, 2016

Do you have good reason to believe the linux patch doesn't break anything? The archscripts target and dependency for __headers was added in commit 6520fe5564 seemingly for a reason. If archscripts are never needed for producing headers that should probably be fixed, but if they're needed for some archs then it's perhaps more complicated. Can you look into this?

The binutils patch looks fine as-is; it matches the upstream fix.

As I believe I noted somewhere else, adding CXX=[anything] to *_CONFIG by default is problematic because it will conflict with settings in the user's config.mak. I think we should consider one of the following 3 approaches:

  1. Just keep the workaround local in your config.mak if you have a compiler that needs it.
  2. Write a patch for GCC's build system that adds -fbracket-depth to its flags or to the command line for just the affected files.
    or
  3. Make some framework where both config.mak and the main Makefile can provide additions/changes to CC, CXX, CFLAGS, etc. without stepping on each other.

While 3 seems nice in some ways, it's also something of a big hammer/almost-gratuitous complexity, so I'd lean towards one of the other options.

@FiloSottile

This comment has been minimized.

Contributor

FiloSottile commented Jun 4, 2016

Do you have good reason to believe the linux patch doesn't break anything? The archscripts target and dependency for __headers was added in commit 6520fe5564 seemingly for a reason. If archscripts are never needed for producing headers that should probably be fixed, but if they're needed for some archs then it's perhaps more complicated. Can you look into this?

At least in 4.4.10, the only archscripts target is x86, which I can build just fine. Also the only tool is relocs, which I really really can't tell how it can be used in install-headers. I suspect adding the archscripts dependency has been either a shortcut or a mistake.

As I believe I noted somewhere else, adding CXX=[anything] to *_CONFIG by default is problematic because it will conflict with settings in the user's config.mak. I think we should consider one of the following 3 approaches:

Yeah, I had misinterpreted. Agreed on 3 being overkill. I'd prefer 2 but I'm running out of time budget for this :) so I'm striking that patch out. I'll leave the issue open for you to decide what to do, and add the line to config.mak in the Homebrew Formula.

@FiloSottile

This comment has been minimized.

Contributor

FiloSottile commented Jun 12, 2016

Ping :) I believe I should have addressed all your above concerns.

Sorry for being pushy, I'm just looking forward to publishing the Homebrew Formula (which by the way would also require #10).

@richfelker richfelker merged commit 8467bf3 into richfelker:master Jun 13, 2016

@richfelker

This comment has been minimized.

Owner

richfelker commented Jun 13, 2016

Sorry, I didn't realize you'd made the changes we discussed. I think it looks good so I'm merging these commits. Thanks!

@FiloSottile

This comment has been minimized.

Contributor

FiloSottile commented Jun 13, 2016

Wonderful, thank you! Let me know what you think of tagging a release (#10).

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