Skip to content

Conversation

cransom
Copy link

@cransom cransom commented Jul 2, 2025

I don't think this is the ideal fix, but I ran into an issue where I
needed to supply other libs to buildInputs for autopatchelf (it was
missing libmusl for libdatadog for my case).

The applyConfig would only apply my gemConfig changes if it intended to
compile them, which looks to be entirely dependent on if there are
multiple targets for the same gem.

I don't think this is the ideal fix, but I ran into an issue where I
needed to supply other libs to buildInputs for autopatchelf (it was
missing libmusl for libdatadog for my case).

The applyConfig would only apply my gemConfig changes if it intended to
compile them, which looks to be entirely dependent on if there are
multiple targets for the same gem.
@cransom
Copy link
Author

cransom commented Jul 2, 2025

diff --git i/modules/gems/expand.nix w/modules/gems/expand.nix
index a711610..705713c 100644
--- i/modules/gems/expand.nix
+++ w/modules/gems/expand.nix
@@ -18,9 +18,9 @@ rec {
     attrs:
     let
       f = gemConfig.${attrs.gemName};
-      apply = (gemConfig ? ${attrs.gemName});
+      apply = (gemConfig ? ${attrs.gemName}) && (attrs.compile || ((f attrs) ? compile && (f attrs).compile));
     in
    if apply then attrs // f attrs else attrs;


   applyGemConfig = _: alts: map applyConfig alts;

@@ -67,9 +67,9 @@ rec {
     let
       sources =
         if attrs.targets == [ ] then
-          singleton (attrs.source // { compile = true; })
+          singleton ({ compile = true; } // attrs.source)
         else
-          map (a: a // { compile = false; }) attrs.targets;
+          map (a: { compile = false; } // a) attrs.targets;
     in
     map (eachSource gemName attrs) sources;

I have this alternate version as well that keeps the compile behavior but allows the user to specify their own compile flag that doesn't get overwritten by ruby-nix. Slightly more verbose for the user, but an option.

Copy link
Owner

@inscapist inscapist left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spotting this, I didn't realized that it's broken all this while!

I think this PR is fine, I prefer it over introducing another point of customization that could be confusing to some. The PR is still in draft, if you have tested it, feel free to merge it in

@cransom cransom marked this pull request as ready for review July 7, 2025 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants