Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pleasant ruby #4677

Merged
merged 71 commits into from
Jan 22, 2015
Merged

Pleasant ruby #4677

merged 71 commits into from
Jan 22, 2015

Conversation

cstrahan
Copy link
Contributor

This is a continuation of @joelteon's excellent work in #3491.

Conflicts:
	pkgs/applications/version-management/git-and-tools/default.nix
	pkgs/applications/version-management/git-and-tools/hub/default.nix
	pkgs/tools/audio/mpdcron/default.nix
@cstrahan
Copy link
Contributor Author

@iElectric @7c6f434c @Phreedom

This is actually almost done - maybe another hour or two of work. The plan is to merge this in tomorrow and send a message to the mailing list.

Concretely, the only thing left to do is allow for sending gem build arguments through bundler (e.g. "--with-xml2-lib=${libxml2}/lib").

It's rather frustrating that this dragged out for so long, but there were a number of subtle challenges going with the general approach that I co-authored with @joelteon and @aflatter. We were serializing the Gemfile.lock as a nix expression, which was (and still is) fantastic, but then we were trying to make the RubyGem resolution work by installing the gems via gem install and manipulating $GEM_PATH, rather than letting bundler do proper loading. This posed a number of problems: as a developer, I couldn't point bundle exec at the build artifacts, frameworks that try to use Bundler.require(:default) completely explode, etc. So a couple days ago I said "screw it, I'll just rewrite the damned thing".

I now have a system in place that hooks into bundler and lets it do the whole install. It's absolutely wonderful, and seems to work really, really well. I'll write up more details in the email I'm going to send tomorrow.

@domenkozar
Copy link
Member

🍻

@cstrahan
Copy link
Contributor Author

Going to merge this in the next hour.

@cstrahan cstrahan changed the title [WIP] Pleasant ruby Pleasant ruby Jan 22, 2015
cstrahan added a commit that referenced this pull request Jan 22, 2015
@cstrahan cstrahan merged commit 8d49e87 into NixOS:master Jan 22, 2015
@peti
Copy link
Member

peti commented Jan 22, 2015

It appears that this change broke evaluation of

  • gist,
  • gitlab,
  • gitlab-shell,
  • gem-nix, and
  • loadRubyEnv.

Just run nix-env -qa to see the errors.

peti added a commit that referenced this pull request Jan 22, 2015
…d loadRubyEnv to fix evaluation

Nixpkgs evaluation was broken in these attributes by the changes from
#4677, I guess.
@peti
Copy link
Member

peti commented Jan 24, 2015

@cstrahan, it's kind-of customary that people who commit a change that breaks other packages make an effort to fix the breakage. I believe that your commit broke gitlab and several other packages (see my previous comment). Do you intend to do anything about that?

@jgeerds
Copy link
Member

jgeerds commented Jan 24, 2015

@cstrahan This breaks also python2.7-pync-1.4

@jgeerds
Copy link
Member

jgeerds commented Jan 26, 2015

@cstrahan Are you working on this? (terminal_notifier)

@cstrahan
Copy link
Contributor Author

@cstrahan, it's kind-of customary that people who commit a change that breaks other packages make an effort to fix the breakage. I believe that your commit broke gitlab and several other packages (see my previous comment). Do you intend to do anything about that?

I'm sorry - I didn't see a GitHub notification for the comments on this thread.

It appears that this change broke evaluation of

  • gist,
  • gitlab,
  • gitlab-shell,
  • gem-nix, and
  • loadRubyEnv.

I resolved those a couple days ago after @iElectric pointed them out to me on IRC. GitLab currently builds, but I still need to test running the service.

@cstrahan Are you working on this? (terminal_notifier)

Yes, I'll have that fixed shortly.

@cstrahan
Copy link
Contributor Author

@cstrahan Are you working on this? (terminal_notifier)

Fixed in:

5a73cda

@jgeerds
Copy link
Member

jgeerds commented Jan 26, 2015

Thanks! 😄

@domenkozar
Copy link
Member

Thanks @cstrahan, appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet