qgis, qgis-ltr: fix dependencies on darwin #157862
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
willcohen
changed the title
|
This does not fully complete the build for darwin, but does fix all of the dependencies. A .app does get produced at this point, but the dylibs still link incorrectly. My preference would be to merge this reduced-scope PR first as a progress step since it deals with the underlying build requirements, and then I can still try to work out fixing the .app bundle's internal issues in a separate PR without having to pull these changes along with, because I don't expect them to change any more than as is. |
|
As context, even the official build recipe for the Mac .app has a whole bunch of steps needed to get the .app bundle working, so some substantial variant of these kinds of steps will probably eventually be needed for the nixpkgs bundle. |
|
Force-pushed to remove darwin as a supported platform (since the .app isn't yet functional), but otherwise the same. |
ofborg
bot
added
11.by: package-maintainer
10.rebuild-darwin: 0
and removed
10.rebuild-darwin: 1-10
labels
|
@ttuegel as I continue working on this, it appears that the qt wrapper is what's being overly zealous, and it's going ahead and trying to wrap not just the executable but the various .dylibs and everything else inside Would you suggest that the best course of action here is to just set |
I would suggest setting |
13 tasks
stale
bot
added
the
2.status: stale
13 tasks
willcohen
force-pushed
the
qgis-darwin
branch
from
acba11f
to
7cf1445
Compare
stale
bot
removed
the
2.status: stale
|
With #209801 merged it's a chance to dust this off. Rebased to master and fixed the dependencies and linking issues from before. However, attempting to load the QGIS.app for either qgis or qgis-ltr fails with the following crash report. Any suggestions @NixOS/darwin-maintainers? |
|
I see source from |
|
Yeah -- it's two different packages. 3.28.2 is |
@willcohen , unfortunately I am not a Mac user at all. But I really want to see it running on Mac because of other users. That's why I am very happy that you try to progress. |
|
Have you tried to build it with debug output ? Something like |
|
.app built with cmake debug crashes with the following, leading me to ask a basic question that's above my knowledge of nix and macos internals -- do these |
My guess will be that it should be nixpkgs provided libs. @NixOS/darwin-maintainers would you please help us with that ? |
|
Per 70f3863 it seems like it's okay to refer to the systemwide one -- my read of this is that something QGIS is calling in Qt is leading to this crashing during the invocation of |
@willcohen , quite old but maybe - https://bugreports.qt.io/browse/QTBUG-48015 |
|
darwin is not completely pure unfortunately. The |
Et7f3
reviewed
Jan 13, 2023
| ] ++ lib.optional (!withWebKit) "-DWITH_QTWEBKIT=OFF" | ||
| ++ lib.optional stdenv.isLinux "-DWITH_PDAL=TRUE" |
There was a problem hiding this comment.
Why this change ? it seems to build fine on Mac with this.
There was a problem hiding this comment.
Good call -- it was broken a long time ago and I just assumed that was still true. Fixed since October with #195216. Will revise.
| $out/Applications/QGIS.app/Contents/Frameworks/qgisgrass8.framework/qgisgrass8 | ||
| do | ||
| echo "Manually running install_name_tool on $f" | ||
| install_name_tool -change "@loader_path/../lib/libqscintilla2_qt5.dylib" "${qscintilla}/lib/libqscintilla2_qt5.dylib" $f |
There was a problem hiding this comment.
how did you get this list ? can it be parsed and looped over ? it should be more resistant to future upgrade
There was a problem hiding this comment.
This was, unfortunately, done by me repeatedly building and running the .app, having it crash due to a linking error, and repeating over and over. I did at least simplify it so it can loop over the dylibs I know about. It seems like the original build script does it with a little less targeting, but it's true that this is moderately fragile. I'm open to suggestions for how to improve it -- fixDarwinDylib names was too blunt an instrument and tried to operate on lots of files in $out, and wasn't able to deal with the Frameworks, so I ended up just doing this manually.
| do | ||
| echo "Manually running install_name_tool on $f" | ||
| install_name_tool -change "@loader_path/../lib/libqscintilla2_qt5.dylib" "${qscintilla}/lib/libqscintilla2_qt5.dylib" $f | ||
| install_name_tool -change "@loader_path/../lib/libqt5keychain.dylib" "${qtkeychain}/lib/libqt5keychain.dylib" $f |
There was a problem hiding this comment.
Suggested change
| install_name_tool -change "@loader_path/../lib/libqt5keychain.dylib" "${qtkeychain}/lib/libqt5keychain.dylib" $f | |
| install_name_tool -change "@loader_path/../lib/libqt5keychain.dylib" "${qtkeychain}/lib/libqt5keychain.dylib" "$f" |
same for all other
There was a problem hiding this comment.
Good catch -- will revise.
I'm not sure that this will ever work with sandbox enabled. Before even anything about tests failing, with sandbox enabled I get |
|
I think that without sandboxing you might get a dependencies that mismatch and it can cause failure. Once I got a build issue because of my locale that wasn't unset. But ok I will un sandbox for this derivation. I was able to launch some test in sandbox mode. |
13 tasks
wegank
added
the
2.status: stale
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
Progress toward #71398. Builds on #150286, #150595, #156747, #156809, #156828.
Things done
@NixOS/darwin-maintainers I'm struggling with getting the final build of this to link and load correctly, though all the dependencies should finally be fixed. Even after fully disabling all of the
makeWrapperwork on darwin, I'm still getting mired down by being unable to get the dylibs to load. Pasting the end result here since building this derivation takes a while.Any suggestions would be MUCH appreciated.
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes