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

keystone: make sure dylib's install name is correct on darwin #217201

Merged
merged 1 commit into from
Mar 15, 2023

Conversation

sternenseemann
Copy link
Member

@sternenseemann sternenseemann commented Feb 19, 2023

Due to an oversight the override ended up in the aarch64-darwin only section of configuration-darwin.nix.

Description of changes
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@sternenseemann
Copy link
Member Author

@ofborg build haskellPackage.bytepatch

@sternenseemann
Copy link
Member Author

@ofborg build haskellPackages.bytepatch

@ofborg ofborg bot added 6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 1-10 10.rebuild-linux: 0 labels Feb 19, 2023
sternenseemann added a commit that referenced this pull request Feb 22, 2023
This reverts commit 32eecc6.

Reason for revert: This fix doesn't alleviate the problem in two ways:

1. It is in the aarch64-darwin section of configuration-darwin.nix,
   the problem appears on x86_64-darwin (bytepatch doesn't support
   aarch64).

2. When moving the override into the general *-darwin section, the
   problem with bytepatch is also unaffected, see my attempt in
   #217201.
@raehik
Copy link
Contributor

raehik commented Feb 22, 2023

That's a bummer to see it wasn't the right fix, I'm a bit helpless on Mac issues. Still gladly help out any way I can but no more ideas from here.

sternenseemann added a commit that referenced this pull request Feb 23, 2023
This reverts commit 32eecc6.

Reason for revert: This fix doesn't alleviate the problem in two ways:

1. It is in the aarch64-darwin section of configuration-darwin.nix,
   the problem appears on x86_64-darwin (bytepatch doesn't support
   aarch64).

2. When moving the override into the general *-darwin section, the
   problem with bytepatch is also unaffected, see my attempt in
   #217201.
@sternenseemann
Copy link
Member Author

@ofborg build haskellPackages.bytepatch

@sternenseemann
Copy link
Member Author

@ofborg build haskellPackages.bytepatch

@sternenseemann sternenseemann marked this pull request as ready for review March 8, 2023 15:21
@sternenseemann sternenseemann changed the title haskellPackages.heystone: apply darwin fix also on x86_64-darwin haskellPackages.heystone: force correct rpath on darwin Mar 8, 2023
@raehik
Copy link
Contributor

raehik commented Mar 8, 2023

A note: I'm baffled that we only see this issue for heystone. It gets libkeystone via a pkgconfig-depends entry in the Cabal file. I assume most other C binding Haskell libs do the same. Is there something wrong with how libkeystone is packaged, or might this Darwin issue also occur elsewhere...?

@sternenseemann
Copy link
Member Author

@raehik Yeah, asking myself the same question as you, but not sure. The rpath business comes up on darwin from time to time, but I don't feel like it should be a problem in this instance — it also doesn't seem to be for other libraries in haskellPackages.

keystone does not have a lot of reverse dependencies in nixpkgs, so maybe there is an issue there, who knows.

@sternenseemann
Copy link
Member Author

@ofborg build haskellPackages.bytepatch

@sternenseemann sternenseemann changed the title haskellPackages.heystone: force correct rpath on darwin keystone: make sure dylib's install name is correct on darwin Mar 13, 2023
@sternenseemann
Copy link
Member Author

@ofborg build haskellPackage.bytepatch

@sternenseemann

This comment was marked as outdated.

Using @rpath leads to problems later on, e.g. when linking against
haskellPackages.heystone which in turn links against keystone itself. As
soon as different rpaths/libdirs are in play, things fall apart.
@sternenseemann

This comment was marked as outdated.

@sternenseemann sternenseemann changed the base branch from haskell-updates to master March 15, 2023 15:04
@sternenseemann
Copy link
Member Author

@ofborg build haskellPackages.bytepatch

@sternenseemann sternenseemann merged commit ae2eac6 into NixOS:master Mar 15, 2023
@sternenseemann sternenseemann deleted the heystone-fix-fix branch March 15, 2023 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants