libappimage: add dev output#524183
Conversation
|
|
||
| postInstall = '' | ||
| # somehow files are installed in both outputs and the fixup hooks fail... | ||
| mv $dev/include/appimage/* $out/include/appimage |
There was a problem hiding this comment.
The includes should be in $dev, no?
There was a problem hiding this comment.
They should, and normally would be, but it looks like their upstream cmake files don't consistently use CMAKE_INSTALL_INCLUDEDIR, so when we specify that on our end we end up with headers in multiple places. I opened a PR to fix that upstream, but I think this workaround is probably fine for now - the headers all get moved to $dev during the fixup phase anyway.
r-burns
left a comment
There was a problem hiding this comment.
LGTM. If you're focused on closure size, you may also want to fix their static/shared libraries - it looks like the package output currently contains static libraries with a _shared suffix (?) and the upstream LIBAPPIMAGE_SHARED_ONLY option doesn't seem to work properly. Maybe AppImageCommunity/libappimage#204 improves the situation, haven't tested it myself.
79d95ca to
cbf320c
Compare
| postFixup = '' | ||
| substituteInPlace $dev/lib/cmake/libappimage/libappimageTargets.cmake \ | ||
| --replace-fail "$out" "$dev" | ||
| ''; |
There was a problem hiding this comment.
Is this still needed after the patch? Also, I haven't tested this, but I suspect the substitution may be a little too aggressive - you don't want to replace $out paths that point to actual $out components, such as the libraries..
Diff on my Plasma
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.