feat(mkLogosModule): auto-stage external_libraries in dev shell#98
Open
danisharora099 wants to merge 1 commit into
Open
feat(mkLogosModule): auto-stage external_libraries in dev shell#98danisharora099 wants to merge 1 commit into
danisharora099 wants to merge 1 commit into
Conversation
When the consumer passes non-empty externalLibInputs, resolve them the same way the package outputs do (via resolveExtInput + mkExternalLib.buildExternalLibs) and emit a shellHook that: - symlinks each resolved cdylib + headers into ./lib so the module's CMakeLists.txt find_library(... PATHS lib NO_DEFAULT_PATH) resolves outside the Nix sandbox (clangd, IDEs, raw cmake) - exports DYLD_LIBRARY_PATH / LD_LIBRARY_PATH / CMAKE_LIBRARY_PATH / CMAKE_INCLUDE_PATH for runtime + cmake search - exports CMAKE_EXPORT_COMPILE_COMMANDS=ON so editor tooling Just Works Gated on extLibValues != [] so modules without external_libraries see no change. Removes the need for downstream modules to hand-roll a devShells.<system>.default override. Verified: nix flake check passes on aarch64-darwin. Amp-Thread-ID: https://ampcode.com/threads/T-019e45fb-5eb1-74ea-8e25-612703031f87 Co-authored-by: Amp <amp@ampcode.com>
This file contains hidden or 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
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.
Closes #97.
What
When the consumer passes a non-empty
externalLibInputs,mkLogosModule's dev shell now:resolveExtInput+mkExternalLib.buildExternalLibspath the package outputs already use).lib/*andinclude/*.hinto./libso the module's existingCMakeLists.txt(find_library(... PATHS lib NO_DEFAULT_PATH)) resolves it outside the Nix sandbox.DYLD_LIBRARY_PATH(Darwin) /LD_LIBRARY_PATH(Linux) +CMAKE_LIBRARY_PATH+CMAKE_INCLUDE_PATH.CMAKE_EXPORT_COMPILE_COMMANDS=ONfor clangd.Why
Every universal C++ module wrapping a Rust
cdylib(or any external lib declared inmetadata.json#nix.external_libraries) currently writes the same dev-shell override to makenix developwork with clangd / IDEs / plaincmake. The most recent example iseth-lez-atomic-swapsPR #26. After this lands, that ~40-line block disappears downstream.Full rationale + design questions in #97.
Backward compatibility
external_libraries: zero change (the new behaviour is gated on the list being non-empty).devShells.<system>.default: keep working.nix buildderivations: untouched. Onlynix developis affected.Verification
nix flake checkpasses onaarch64-darwin(all 4 checks, includingdevShells.aarch64-darwin.default).+62 / −2.Notes for review
A few small calls in here I'd like a second opinion on (also in #97):
./lib. Using symlinks here so a cdylib rebuild is picked up on the next shell entry; the in-sandboxcopyExternalLibsToLibusescp. Worth surfacing.*.honly — happy to extend to*.hpp/ extension-less outputs if anyone needs it.externalLibInputsis non-empty" vs an explicitautoStageExternalLibstoggle. I went with implicit.resolveExtInputis duplicated locally insidedevShellsrather than hoisted, to keep the blast radius small. Happy to lift it as a follow-up if reviewers prefer.