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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these needed because
rubydepends on them? If so, isn't that something that should be baked into the requirements of therubyderivation?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libyamlbecause ofpsychandzlibbecause offaraday-gzipthat depends on thezlibgem. If those gems weren't being required in the Gemfile.lock those packages would not be needed, so no, I don't think it should be in the ruby derivation.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But
psychandzlibare both default gems, so they should be coming with a Ruby installation, and their dependencies should also be pulled in by default, no? (I am not sure if thezlibgem has a dependency on thezlibpackage, butpsychshould definitely be pullinglibyamlin)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
libyamlandzlibas just needed to build Ruby. After it is built you don't need it anymore. You only need again if you aren't using the Ruby default gems and is building it from source again, which is the case here, no? Now, why rubygems is building from source and not using the default gem I don't know. Maybe different versions?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't mean to hold up this PR for this, we can discuss async as well, but my understanding of the Nix store was that dependencies of things that you installed persisted in the store, so I was expecting at least
libyamlto be present after installingruby. But, maybe my assumptions are wrong.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that is what I thought. The derivation has it as build packages. After it is built it is gone https://github.com/bobvanderlinden/nixpkgs-ruby/blob/master/ruby/package-fn.nix#L76-L79.
Maybe we should add to our own ruby flake, but we don't have one right now, AFAIK