BundlerEnv, now with groups and paths #25980
Conversation
Not every gem package uses pname, nor should it.
`usesGemspec` no longer required to trigger the "copy everything into gemfile-and-lock" behavior. If the mainGem is referred to by path, that's sufficient.
The bin stubs need to be built where there's access to /nix/store - so it can't happen in a nix-shell run. Ergo, a shell.nix needs to be able to signal to the build that all bins need to be built.
…e when groups match in use.
…e when platform match in use.
|
One driver of a new PR was @zimbatm's comment that the existing bundlerEnv was already pretty complicated. As I've been working through this again, I think that much of that complexity is inherent - or at least external to Nix. One of the core difficulties is that, if you're trying to install a gem-ified tool and it's specific requirements, there's kind of two ways in Rubyland to do that: the A user of a Ruby gem, though, expects for binaries to be installed as described by the gemspec. The What I contemplate as a follow on is a rubyTool builder with explicit |
|
Also in this PR: a ton of (still slightly messy) testing support. I found I wasn't able to keep all the concerns involved here straight between the occassional work sessions I've been able to make for it. After the fact I found |
|
thanks @nyarly, I will take a closer look this weekend |
| drvName = | ||
| if name != null then name | ||
| else if pname != null then "${toString pname}-${mainGem.version}" | ||
| if name != null then lib.traceVal name |
| in | ||
| bundlerEnv | ||
| if pname == null then | ||
| basicEnv // { inherit name basicEnv; } |
There was a problem hiding this comment.
I'm not sure if the inherit name will also change the derivation output name.
| # out. Yes, I'm serious. | ||
| confFiles = runCommand "gemfile-and-lockfile" {} '' | ||
| mkdir -p $out | ||
| ${maybeCopyAll pname} |
There was a problem hiding this comment.
that would mean that the dependencies would be re-built is any of the project files are changing right?
| rec { | ||
| filterGemset = {ruby, groups,...}@env: gemset: lib.filterAttrs (name: attrs: platformMatches ruby attrs && groupMatches groups attrs) gemset; | ||
|
|
||
| platformMatches = {rubyEngine, version, ...}@ruby: attrs: ( |
| ) attrs.platforms | ||
| ); | ||
|
|
||
| groupMatches = groups: attrs: ( |
|
|
||
| testDirective = report: ""; | ||
|
|
||
| testYaml = report: ""; |
| ${lib.escapeShellArg groups} | ||
| ''; | ||
|
|
||
| pathDerivation = { gemName, version, path, ... }: |
There was a problem hiding this comment.
Tell me if I misunderstood the purpose of this function.
When a gem is specified by path, all the files in the repo get copied into a derivation with the Gemfile. Now it would be a shame to make another copy of the gem into a new derivation. So instead this function is returned which pretends to be a derivation output but actually points into the other derivation. path would be something like /nix/store/...-foobar/vendor/gems/mygem.
| outputs = [ "out" ]; | ||
| out = res; | ||
| outputName = "out"; | ||
| }; |
There was a problem hiding this comment.
for even better derivation simulation, add __toString = self: self.outPath;. That is a magic function that is used when coercing the attribute set into a string.
|
|
||
| name = if name == null then pname else name; | ||
|
|
||
| #name = pname; |
| , bundler | ||
| }@defs: | ||
|
|
||
| { |
There was a problem hiding this comment.
it would be helpful to describe the responsibilities of the function since it's so complicated
…bundlerenv_usecases
|
|
||
| testLine = report: "${okStr report} ${toString (report.index + 1)} ${report.description}" + testDirective report + testYaml report; | ||
|
|
||
| # These are part of the TAP spec, not yet implemented. |
There was a problem hiding this comment.
Open an issue for this problem and reference the opened issue here.
|
I've added docs about bundlerApp - it seems presumptuous at this point to say "bundlerEnv is deprecated" - especially since there are several derivations that use it. Once this lands, perhaps a survey to see if bundlerEnv can be replaced by bundlerApp and if not, what shortcomings exist to be addressed. |
doc/languages-frameworks/ruby.xml
Outdated
| }; | ||
| }]]> | ||
|
|
||
| <para>The chief advantage of <literal>bundlerApp</literal> over <literal>bundlerEnv</literal> is the the executables introduced in the environment are precisely those selected in the <literal>exes</literal> list, as opposed to <literal>bundlerEnv</literal> which adds all the executables made available by gems in the gemset, which can mean e.g. <command>rspec</command> or <command>rake</command> in unpredicable versions available from various packages. |
doc/languages-frameworks/ruby.xml
Outdated
|
|
||
| <para>The chief advantage of <literal>bundlerApp</literal> over <literal>bundlerEnv</literal> is the the executables introduced in the environment are precisely those selected in the <literal>exes</literal> list, as opposed to <literal>bundlerEnv</literal> which adds all the executables made available by gems in the gemset, which can mean e.g. <command>rspec</command> or <command>rake</command> in unpredicable versions available from various packages. | ||
|
|
||
| <para>Resulting derivations for both builders also have two helpful items, <literal>env</literal> and <literal>wrapper</literal>. The first one allows one to quickly drop into |
|
Anything else here? |
|
@nyarly Thanks for the efforts! I don't want to impede progress; however, I think the present state of the ruby/bundler stuff in Nixpkgs is plenty tricky (and I say that as the principal author), and I think some tweaks could be made to the PR to stave off further trickiness. Here are the things that I find most excellent:
Some of my concerns:
If we can address those few concerns, I'd love to get this merged in. I understand that, if we don't share the same aesthetic, it might be a bit bothersome to play "guess what would please @cstrahan", so if it's alright with you, I think I'll try to make the changes that I'd like and run them by you (hopefully over the next day or two). Alternatively, if you'd like to take a stab at this, I can make myself available here or on IRC. |
|
Hey @cstrahan - thanks for the feedback. I'm glad you're pleased by the main thrust here - constraining exes, selecting groups. Perhaps not obvious is that you're also able to use the (closer to) the full breadth of Bundler's Gemfile (e.g. the gemspec command) in nix-shell. To your specific objections: Regarding the TAP stuff: I quickly found that the behaviors I was trying to add were more complicated that I could reasonably hand test. My Ruby-land instincts were to do as much TDD as I could manage (which I assume you can identify with), and queries in IRC came up with a kind of general shrug. I've since found that there's stuff in lib to do testing - if I'd known I wouldn't have written the TAP stuff! That said, I feel like there is value in the stuff I've started here, if only because the existing testing framework targets the integration level. With regards to it being half baked, I assume you're referring to the YAML stuff - it's an optional part of TAP which (if this framework were adopted more generally) it would be worth putting in the elbow grease to build YAML output etc. (If it isn't obvious, I do hope it is the start of something - testing in Nix was really pleasant.) Regarding TAP as a separate PR: maybe... but the motivation was the work in this PR, and on it's own I wonder if the utility of the testing framework would be evident. There's something of a chicken-and-egg problem there. Regarding the size: while I agree that it's not a small PR, there's actually a fair amount going on. I realize in retrospect, I bit off an awful lot in starting this - although I contend there's not so many of us with the requisite expertise with Bundler, Rubygems et al. Better to have built a few more "package was missing or outdated" PRs first. I appreciate your empathy about all this. This PR is months in the making, and does directly serve needs I have related to doing Ruby development work in Nix - one of the things that first drew me to Nix{,pkgs,OS} in the first. (I'm trying to contribute to RDF::Shex, which has moved on in the meantime...) So while there is the well-spotted "guess what will please @cstrahan" problem, there's also "please @zimbatm" and a handful of other contributors and the meta-game of "guess which contributor you need to please to get the PR merged" :) Maybe the thing to do is an RFC on the mailing list? (sadly you cannot see me flinch as I suggest that) |
|
@zimbatm @cstrahan I'd like to make progress on this, but at this point I'm at a loss about how to proceed. It seems like, on the face of it, there's two potential approaches to adding the features that I need: extend the interface and function of the existing bundlerEnv (which is what #23000 did), or modularize into more, smaller components. My understanding over the last few months is that @cstrahan favors the former and @zimbatm the latter. Is there a way we can resolve this and get this merged? |
Yikes, 17 days ago! With the way the last couple weeks have gone down, it feels more like half a week, at most. I feel really bad for leaving you hanging! I'm reading through #23000 and also reviewing your feedback. Gimme a sec. |
Ah. I didn't realize that I was effectively pulling you in two directions.
Yeah, I totally appreciate that. I think that's an awesome initiative, and I definitely want to see some push-button approach to vetting that the whole thing works as expected.
Definitely! My only concern was that if it wasn't completely functional (and maybe it is -- feel free to correct me), it might add to the cognitive overhead of getting involved in this corner of Nixpkgs. I've been burned many times from jumping into a project where I crawl into some corner, take a look around and then start building up a mental model for the common vernacular, and the what-does-what and what-goes-where, only for someone (all to late!) to exclaim "oh, don't look there, that's <dead-code / future-but-not-integrated-work / whatever>!" So this quite possibly was a knee-jerk reaction, and it may not have been warranted (apologies!).
I know how it can be. Getting the initial bits of the Bundler support in place was incredibly taxing (my PR to add SHAs to Rubygems.org languished due to them deciding to rewrite the git history right around the same time, and suggestions to Bundler to make things more amenable to packaging fell on deaf ears). I presume you're getting a taste of that here, and I apologize for that. I hope (and I suspect) that things will go little more smoothly in future endeavors. Sadly, interest in Ruby is a little underrepresented in the Nix community (though certainly non-zero), which I think is another chicken-and-egg thing: if the Ruby stuff is painful, why would any Rubyists want to invest in improving any of it? I hope things will improve and that things will no longer be blocked on just me and/or @zimbatm, and as such, your interest - and more importantly, involvement - is very, very much appreciated!
A confession: I haven't read a single personal email in about 2 months. Sounds crazy, I know, but I figure if it's important, someone will either ring me or meet me in meat-space. I've been really strung out and oversubscribed, and something - for the sake of my sanity - had to give. Email - what with the incessant recruiter spam, shitty product ads, Yelp/Amazon/dozens-of-similar-ilk alerts that consistently evade my filters (they keep changing the domain names and such; none of them obey my request to unsubscribe; and I can't mark them as spam or I'll risk not receiving the shit I do want, like order receipts) - was just the thing I needed a break from. Thus, I'm now a proud "inbox 3K" person, and it feels great. I'll probably have to get back to reality at some point, but for now I'm going to savor it. In short, I had no idea you emailed the mailing list 😜. Alright, so about the fate of this PR. I really want all this stuff in Nixpkgs - the only thing that I have the slightest concern with is the shape of the thing. Function-wise, it's solid 👍. So here's where I feel bad -- I actually really like the idea of extending the underlying So here's my thinking (i.e. TL;DR): instead of pulling you in two directions, lets accept this now. The onus will then be on me to submit any subsequent PRs for your and @zimbatm's review (and any other interested parties, should they exist). We have about a month until the 17.09 branch-off, so I'll implement and propose any breaking changes within that window. That should get your changes in and avoid bit-rot, and hopefully feel a lot more fair than the current state of things. The one bit of possible difficulty on your part will (potentially) be upkeep with any changes that make it in between now and the branch off; it's my hope, though, that this makes for a decent compromise on the whole. How does that sound? If that's all good, I'll go ahead and pull the trigger. |
|
@cstrahan You have my sympathies - I know how things can get to be that way (i.e. avoiding email etc), and it usually sucks. I'm 100% behind merging this and working forward on it. I look forward to seeing the next iteration. I would like to have the conversation about expanding bundlerEnv vs. splitting it based on use case etc. Something to find time with @zimbatm on, if we can. Separately, I'd love to expand the question of testing - totally agree that the TAP stuff should either conquer or die, and the "oh, not that code" is exactly the guiding principle. If it dies, I would really like there to be a replacement though. (I do think that the testing layer itself is reasonably functional - there's some shell scripts to drive it, but they're just shims around nix-build). Who can we bring in on that? |
Sweet!
Sounds good. I'll try to make myself available over IRC over the next week or so (I get push-notifications to my phone, so that's actually a pretty reliable way to get a hold of me, assuming I'm not asleep when the notification arrives).
Not sure about who to bring in. @pikajude has done a bunch of great stuff Ruby-wise, but I haven't seen him in a while, so I don't know how active he is / wants to be. There's the chance that I can help out, but... we both saw how well that worked out here 😉. Other people might be more responsive on the mailing list, so that might be a good way to drum up support. |
|
This looks good to me, I wish I'd had this 6 months ago when I was doing Ruby :D |
|
I think the manual stopped building after this, see https://travis-ci.org/NixOS/nixpkgs/jobs/258787424 |
|
nixpkgs manual build fixed: 4d66de8 |
|
@fpletz thanks :) |
|
Looks like there might be some problems still: https://travis-ci.org/NixOS/nixpkgs/jobs/258903666 |
|
Fixed up further in 0154d1b, could you check if I interpreted everything correctly? |
This is a retry of
#23000
A crucial difference is that most of the machinery to assemble
a bundler working environment is now part of basic.nix,
and bundlerEnv uses that and then
binstubs in the specific gem it's being built for.
This is preliminary,
in that I'm thinking that there should also be
a builder expression to build a Gemfile
and then put links to explicit executables into $out.
At the moment though, I think this could be merged
as is:
it replaces all the functionality of bundlerEnv
and lays the groundwork for other things.