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

BUNDLED WITH breaks git bisect run #3726

Closed
chastell opened this Issue Jun 6, 2015 · 10 comments

Comments

Projects
None yet
4 participants
@chastell
Contributor

chastell commented Jun 6, 2015

The recent addition of BUNDLED WITH section to Gemfile.lock breaks git bisect run:

  1. Have a good and a bad commit, marked appropriately with git bisect, that have different (or missing) BUNDLED WITH sections in Gemfile.lock.
  2. Have a script foo that verifies whether a problem exists (and runs bundle install when necessary).
  3. Run git bisect run foo.
  4. The automatic Git bisecting workflow breaks as Gemfile.lock is modified by the first bundle install run.

A possible (albeit cumbersome) workaround would be to make the foo script reset Gemfile.lock after each run, but that was not necessary with Bundler 1.9 (and earlier) and, arguably, shouldn’t be the foo script’s responsibility. Given the above I would say that adding BUNDLED WITH is a backwards-incompatible change, as it breaks perfectly valid usage of other tools. (I understand that you need this for the next major release of Bundler, so maybe 1.10 should be renamed to 2.0 and the next major release should be 3.0?)

Can the BUNDLED WITH information be kept outside Gemfile.lock, e.g. in .bundle/bundled_with?

(I don’t think this is a duplicate of #3697, as its resolution does not address this problem.)

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Jun 6, 2015

Member

If you don't want the possibility of bundler changing your lockfile at all, you'll need to run with the --deployment flag

Member

segiddins commented Jun 6, 2015

If you don't want the possibility of bundler changing your lockfile at all, you'll need to run with the --deployment flag

@simi

This comment has been minimized.

Show comment
Hide comment
@simi

simi Jun 6, 2015

Contributor

I think only bundle update and bundle install (when Gemfile.lock is not present) should change Gemfile.lock.

Contributor

simi commented Jun 6, 2015

I think only bundle update and bundle install (when Gemfile.lock is not present) should change Gemfile.lock.

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Jun 6, 2015

Member

@simi that change will be made in #3722

Member

segiddins commented Jun 6, 2015

@simi that change will be made in #3722

@simi

This comment has been minimized.

Show comment
Hide comment
@simi

simi Jun 6, 2015

Contributor

Ahh sorry @segiddins. I missed that. Great work!

Contributor

simi commented Jun 6, 2015

Ahh sorry @segiddins. I missed that. Great work!

@chastell

This comment has been minimized.

Show comment
Hide comment
@chastell

chastell Jun 7, 2015

Contributor

If you don't want the possibility of bundler changing your lockfile at all, you'll need to run with the --deployment flag

This does not address the use case above, as (a) this installs gems to vendor/bundle and (b) it still means the foo script ran by git bisect run needs to be changed for Bundler 1.10 (which means adding BUNDLED WITH is a backwards-incompatible change). Also the docs explicitly say Do not use this flag on a development machine.

I think only bundle update and bundle install (when Gemfile.lock is not present) should change Gemfile.lock.

Yes, but this does not address the use case above – the script ran by git bisect run does bundle install (for a valid reason) and automatic Git-bisecting works perfectly fine in Bundler 1.9, while Bundler 1.10 breaks it altogether (and requires changing the script to make it work again).

Is there a reason why the version information needs to be kept in Gemfile.lock and not somewhere else?

(I’m aware that you must be dead tired by the BUNDLED WITH issues by now – and I’m eternally thankful for Bundler, which is one of the reasons why I’m a Ruby Together member – but please understand that adding this particular feature in a minor release does break other people’s workflows.)

Contributor

chastell commented Jun 7, 2015

If you don't want the possibility of bundler changing your lockfile at all, you'll need to run with the --deployment flag

This does not address the use case above, as (a) this installs gems to vendor/bundle and (b) it still means the foo script ran by git bisect run needs to be changed for Bundler 1.10 (which means adding BUNDLED WITH is a backwards-incompatible change). Also the docs explicitly say Do not use this flag on a development machine.

I think only bundle update and bundle install (when Gemfile.lock is not present) should change Gemfile.lock.

Yes, but this does not address the use case above – the script ran by git bisect run does bundle install (for a valid reason) and automatic Git-bisecting works perfectly fine in Bundler 1.9, while Bundler 1.10 breaks it altogether (and requires changing the script to make it work again).

Is there a reason why the version information needs to be kept in Gemfile.lock and not somewhere else?

(I’m aware that you must be dead tired by the BUNDLED WITH issues by now – and I’m eternally thankful for Bundler, which is one of the reasons why I’m a Ruby Together member – but please understand that adding this particular feature in a minor release does break other people’s workflows.)

@segiddins

This comment has been minimized.

Show comment
Hide comment
@segiddins

segiddins Jun 8, 2015

Member

I don't think we've ever made a promise that running bundle install won't modify your Gemfile.lock

Member

segiddins commented Jun 8, 2015

I don't think we've ever made a promise that running bundle install won't modify your Gemfile.lock

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Jun 8, 2015

Member

Yeah, I think there's a couple of things going on here.

First, @chastell, I appreciate that you opened a ticket trying find a solution to your git bisect problem, rather than to complain. :) Second, as @segiddins pointed out, running bundle install has the potential to change your lockfile, even before 1.10 was released.

For the record, this change does not break backwards compatibility for Bundler. Version 1.10 will still install gems in exactly the same manner as version 1.0. However, there are some existing git bisect scripts that do not know how to handle a change to the lock file during the testing process. (This is actually a problem for more than just the 1.9/1.10 change: any commit that contains a change to the Gemfile without also committing the matching lock will have this problem as well.)

Since this problem applies to all applications, and the 1.10 release just makes the need to handle this situation more common, let's see if there is a shared solution that everyone can use in their git bisect scripts. It turns out that this situation is so common that it is explicitly described in the man page for git bisect, which states:

To cope with such a situation the script can […] run the real test […] and then rewind the tree to the pristine state. Finally the script should exit with the status of the real test to let the "git bisect run" command loop determine the eventual outcome of the bisect session.

This approach actually solves a more general problem—how can you git bisect when running the test might dirty the git tree? The only way to ensure that git bisect can run fully automatically is to explicitly clean up any files that might be changed as the test is run.

In a hypothetical bisecting situation where the failing test being bisected can be run with bin/rake spec, here is a git bisect script that should work completely automatically:

#!/bin/bash
bundle install
bin/rake spec
status=$?
git reset --hard HEAD
exit $status

I've created a ticket to document the process for bisecting while using Bundler over at bundler/bundler-site#160. Thanks for bringing this up!

Member

indirect commented Jun 8, 2015

Yeah, I think there's a couple of things going on here.

First, @chastell, I appreciate that you opened a ticket trying find a solution to your git bisect problem, rather than to complain. :) Second, as @segiddins pointed out, running bundle install has the potential to change your lockfile, even before 1.10 was released.

For the record, this change does not break backwards compatibility for Bundler. Version 1.10 will still install gems in exactly the same manner as version 1.0. However, there are some existing git bisect scripts that do not know how to handle a change to the lock file during the testing process. (This is actually a problem for more than just the 1.9/1.10 change: any commit that contains a change to the Gemfile without also committing the matching lock will have this problem as well.)

Since this problem applies to all applications, and the 1.10 release just makes the need to handle this situation more common, let's see if there is a shared solution that everyone can use in their git bisect scripts. It turns out that this situation is so common that it is explicitly described in the man page for git bisect, which states:

To cope with such a situation the script can […] run the real test […] and then rewind the tree to the pristine state. Finally the script should exit with the status of the real test to let the "git bisect run" command loop determine the eventual outcome of the bisect session.

This approach actually solves a more general problem—how can you git bisect when running the test might dirty the git tree? The only way to ensure that git bisect can run fully automatically is to explicitly clean up any files that might be changed as the test is run.

In a hypothetical bisecting situation where the failing test being bisected can be run with bin/rake spec, here is a git bisect script that should work completely automatically:

#!/bin/bash
bundle install
bin/rake spec
status=$?
git reset --hard HEAD
exit $status

I've created a ticket to document the process for bisecting while using Bundler over at bundler/bundler-site#160. Thanks for bringing this up!

@indirect indirect closed this Jun 8, 2015

@chastell

This comment has been minimized.

Show comment
Hide comment
@chastell

chastell Jun 8, 2015

Contributor

It turns out that this situation is so common that it is explicitly described in the man page for git bisect

Well, actually ;) the man page describes a situation where the script needs to alter the state of the tree for a reason, not when one of the tools it uses suddenly starts to make the tree dirty:

You may often find that during a bisect session you want to have temporary modifications (e.g. s/#define DEBUG 0/#define DEBUG 1/ in a header file, or "revision that does not have this commit needs this patch applied to work around another problem this bisection is not interested in") applied to the revision being tested.

– but ok, let’s not split hairs here (also: thanks for taking the time to write the example script!).

I’m still not sure why the version information can’t be put basically anywhere else than in Gemfile.lock (in which case I could either start committing that other file – and it would be my scripts’ responsibility to handle it – or just git-ignore it and live with the consequences), but after asking twice already I fear it’s some kind of a taboo question, so I’ll refrain from pressing it any more. ;)

Contributor

chastell commented Jun 8, 2015

It turns out that this situation is so common that it is explicitly described in the man page for git bisect

Well, actually ;) the man page describes a situation where the script needs to alter the state of the tree for a reason, not when one of the tools it uses suddenly starts to make the tree dirty:

You may often find that during a bisect session you want to have temporary modifications (e.g. s/#define DEBUG 0/#define DEBUG 1/ in a header file, or "revision that does not have this commit needs this patch applied to work around another problem this bisection is not interested in") applied to the revision being tested.

– but ok, let’s not split hairs here (also: thanks for taking the time to write the example script!).

I’m still not sure why the version information can’t be put basically anywhere else than in Gemfile.lock (in which case I could either start committing that other file – and it would be my scripts’ responsibility to handle it – or just git-ignore it and live with the consequences), but after asking twice already I fear it’s some kind of a taboo question, so I’ll refrain from pressing it any more. ;)

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Jun 8, 2015

Member

Basically, It has to be in the lock specifically to keep you from being able to git ignore it :P We need that information checked in to the repo in order to support Bundler 2.0 when it comes out, and putting it in another file would mean it could be ignored or out of date from the Gemfile and lock. It's not fantastic, but it's the best option out of the tradeoffs that we had.

Member

indirect commented Jun 8, 2015

Basically, It has to be in the lock specifically to keep you from being able to git ignore it :P We need that information checked in to the repo in order to support Bundler 2.0 when it comes out, and putting it in another file would mean it could be ignored or out of date from the Gemfile and lock. It's not fantastic, but it's the best option out of the tradeoffs that we had.

@indirect

This comment has been minimized.

Show comment
Hide comment
@indirect

indirect Jun 24, 2015

Member

We've added workarounds for BUNDLED WITH issues in 1.10.4 and 1.10.5, and the BUNDLED WITH section will no longer be added or updated during bundle install if there are no other changes to the lock. (Anyone who explicitly wants to update the BUNDLED WITH section can run bundle update --source bundler --local.) Read more about it on the Bundler blog.

Member

indirect commented Jun 24, 2015

We've added workarounds for BUNDLED WITH issues in 1.10.4 and 1.10.5, and the BUNDLED WITH section will no longer be added or updated during bundle install if there are no other changes to the lock. (Anyone who explicitly wants to update the BUNDLED WITH section can run bundle update --source bundler --local.) Read more about it on the Bundler blog.

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