Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

[Installer] Don't eval gemfile twice #4417

Closed
wants to merge 2 commits into from

Conversation

segiddins
Copy link
Member

Closes #3096

\c @indirect

@segiddins
Copy link
Member Author

I don't know how to make this work, it turns out .duping a SourceList doesn't do what we need, and because there's hidden mutable state in Source and we need a matching source because @dependencies maintains references to sources and 😭

@segiddins segiddins self-assigned this Apr 17, 2016
@segiddins
Copy link
Member Author

Can anyone think of any possible avenues to make this work?

@indirect
Copy link
Member

I have previously spent days fighting this and eventually given up. I think we need less hidden mutable state for it to be possible?

@segiddins
Copy link
Member Author

@indirect well... yes. But a rewrite of how source / index / definition work isn't really feasible given time constraints right now

@colby-swandale
Copy link
Member

Im happy to work on this @segiddins if you're on other things.

@segiddins
Copy link
Member Author

@colby-swandale up to you -- it might just be frustrating though, so I can't wholeheartedly suggest it :/

@indirect indirect mentioned this pull request Aug 11, 2016
23 tasks
@homu
Copy link
Contributor

homu commented Aug 25, 2016

☔ The latest upstream changes (presumably #4654) made this pull request unmergeable. Please resolve the merge conflicts.

homu added a commit that referenced this pull request Sep 10, 2016
[Installer] Skip eval’ing a Gemfile twice when there’s a lockfile

Closes #3096. Closes #4417.

\c @indirect
@segiddins
Copy link
Member Author

Done in 2.0

@coilysiren
Copy link
Contributor

Closed via #4952

@coilysiren coilysiren closed this Oct 9, 2016
@segiddins segiddins deleted the seg-no-eval-gemfile-twice branch June 13, 2017 19:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants