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

[Lockfile] Store each rubygems remote separately#3593

Closed
segiddins wants to merge 41 commits into2-0-devfrom
segseparate-lockfile-sources
Closed

[Lockfile] Store each rubygems remote separately#3593
segiddins wants to merge 41 commits into2-0-devfrom
segseparate-lockfile-sources

Conversation

@segiddins
Copy link
Copy Markdown
Contributor

Closed https://trello.com/c/JlXm2yZy/100-split-gem-server-sources-in-the-lock-file.

I don't this is done yet, but I want to go to bed.

\c @TimMoore @indirect

segiddins and others added 21 commits March 3, 2015 13:34
Remove deployment.rb and vlad.rb
Invert defaults for caching during install
This solution uses a less-than-ideal method to add Thor-formatted printing to STDERR. Because Thor doesn't offer this out-of-the-box, I've used Thor's internals to format a message and pass it to Thor's stderr.

Since this uses protected methods, changes under the hood in Thor may break this. If this happens, tests will fail.

I also added basic tests to ensure other print methods from `Bundler::UI::Shell` continue printing to STDOUT.
…s-1-99-dev

Edit bundle-config.ronn and bundle-install.ronn re: previously remembered flags
@indirect
Copy link
Copy Markdown

Do we need to explicitly opt this in to travis? Or just push a new commit now that 2-0-dev is on the list?

[SourceList] Warn when adding a git source using the git procotol
@segiddins segiddins closed this May 1, 2015
@segiddins segiddins reopened this May 1, 2015
@indirect
Copy link
Copy Markdown

indirect commented May 1, 2015

All fails, so I guess this PR still needs some work. :)

@segiddins
Copy link
Copy Markdown
Contributor Author

@indirect pretty sure 2-0-dev is all fails to begin with :p

@indirect
Copy link
Copy Markdown

indirect commented May 1, 2015

Oh we should probably fix that then :P

@segiddins segiddins force-pushed the segseparate-lockfile-sources branch from e99aa11 to ec289da Compare May 17, 2015 00:49
@segiddins
Copy link
Copy Markdown
Contributor Author

I don't really see how I can make this work, given the rubygems aggregate exists. Maybe marking the aggregate as such as an entry in the options hash?

@indirect
Copy link
Copy Markdown

I think the endgame is to get rid of the aggregate, but I have no idea how hard that is right now.

On Sat, May 16, 2015 at 8:32 PM, Samuel E. Giddins
notifications@github.com wrote:

I don't really see how I can make this work, given the rubygems aggregate exists. Maybe marking the aggregate as such as an entry in the options hash?

Reply to this email directly or view it on GitHub:
#3593 (comment)

@TimMoore
Copy link
Copy Markdown
Contributor

Yeah, one of the main reasons the rubygems aggregate exists is because the remotes are not stored separately in the lock file. Hopefully, with this change the aggregate can be removed.

Another reason is to handle multiple top-level sources. Should we disallow that entirely in 2.0? Even if we keep allowing it, it would be good to refactor Source::Rubygems to be single-source only, and handle multiple top-level sources with something like composite pattern.

@indirect
Copy link
Copy Markdown

I would strongly prefer that we stop allowing multiple top level sources in 2.0. It's already opt-in as of 1.8 via config, and we've said it will become an error in 2.0.

On Mon, May 18, 2015 at 2:42 PM, Tim Moore notifications@github.com
wrote:

Yeah, one of the main reasons the rubygems aggregate exists is because the remotes are not stored separately in the lock file. Hopefully, with this change the aggregate can be removed.

Another reason is to handle multiple top-level sources. Should we disallow that entirely in 2.0? Even if we keep allowing it, it would be good to refactor Source::Rubygems to be single-source only, and handle multiple top-level sources with something like composite pattern.

Reply to this email directly or view it on GitHub:
#3593 (comment)

@TimMoore
Copy link
Copy Markdown
Contributor

👍 that should simplify things a lot. Then all rubygems sources have a single remote, and the only special thing about the top level one (after the Gemfile is eval'ed) is that it acts as a fall back source for transitive dependencies that aren't found in the source of the parent gem.

@segiddins
Copy link
Copy Markdown
Contributor Author

Then how do we mark the top level one as such?

-Samuel E. Giddins

On May 18, 2015, at 4:47 PM, Tim Moore notifications@github.com wrote:

that should simplify things a lot. Then all rubygems sources have a single remote, and the only special thing about the top level one (after the Gemfile is eval'ed) is that it acts as a fall back source for transitive dependencies that aren't found in the source of the parent gem.


Reply to this email directly or view it on GitHub.

@TimMoore
Copy link
Copy Markdown
Contributor

Then how do we mark the top level one as such?

Do you mean in memory at runtime, or in Gemfile.lock?

For the former, we could still have a special instance variable in SourceList that points to the top-level source.

For the latter, do we need to?

@segiddins
Copy link
Copy Markdown
Contributor Author

Yes, we need to for the latter, or we’ll need to make major changes to how we detect if the sources in the definition have changed.

On May 18, 2015, at 7:01 PM, Tim Moore notifications@github.com wrote:

Then how do we mark the top level one as such?

Do you mean in memory at runtime, or in Gemfile.lock?

For the former, we could still have a special instance variable in SourceList that points to the top-level source.

For the latter, do we need to?


Reply to this email directly or view it on GitHub #3593 (comment).

@TimMoore
Copy link
Copy Markdown
Contributor

Well, we could add a line like default: true to the GEM section, but I would have thought that we'd be able to detect changes just by looking at which gems come from which sources.

@segiddins
Copy link
Copy Markdown
Contributor Author

But there will still be some rearrangements that we wouldn't be able to detect? I think....

-Samuel E. Giddins

On May 18, 2015, at 9:09 PM, Tim Moore notifications@github.com wrote:

Well, we could add a line like default: true to the GEM section, but I would have thought that we'd be able to detect changes just by looking at which gems come from which sources.


Reply to this email directly or view it on GitHub.

@TimMoore
Copy link
Copy Markdown
Contributor

It's possible (like maybe if you swap a source block with the top level) but maybe it isn't relevant because the behaviour would be the same?

@indirect
Copy link
Copy Markdown

@segiddins any interest in finishing this off, or should we leave it for @smlance if he has time?

@segiddins
Copy link
Copy Markdown
Contributor Author

@indirect I probably can, just need to get rid of the aggregate

@segiddins
Copy link
Copy Markdown
Contributor Author

@TimMoore any chance you want to take this over? Or should I just close?

@indirect
Copy link
Copy Markdown

I'm happy to get this done myself as soon as GSoC stuff is taking up less of my time.

On Mon, Aug 17, 2015 at 8:20 PM, Samuel E. Giddins
notifications@github.com wrote:

@TimMoore any chance you want to take this over? Or should I just close?

Reply to this email directly or view it on GitHub:
#3593 (comment)

@TimMoore
Copy link
Copy Markdown
Contributor

I should have some time this weekend.

@segiddins
Copy link
Copy Markdown
Contributor Author

@TimMoore is it worth keeping this open, or would we be better off starting from scratch?

@TimMoore
Copy link
Copy Markdown
Contributor

Oh wow, where have the months gone? Let's close it and reassess.

@TimMoore TimMoore closed this Jan 31, 2016
chalkos added a commit to chalkos/bundler that referenced this pull request Jun 12, 2016
chalkos added a commit to chalkos/bundler that referenced this pull request Jun 12, 2016
@segiddins segiddins deleted the segseparate-lockfile-sources branch August 15, 2016 20:08
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.

5 participants