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

Make locking the Gemfile optional on $ bundle check #1811

Closed
wants to merge 1 commit into from

Conversation

svenfuchs
Copy link
Contributor

Currently bundle check locks the Gemfile. According to @indirect this
behaviour is intentional (see
https://twitter.com/#!/indirect/status/186493694013210624).

I am working on a CLI monitoring tool that makes it easier to work with
cross-repository changes on projects that are split up into several
repositories.

This tool is supposed to display status information such as out-of-date
bundler Gemfiles etc. To do so it watches the filesystem and queries for
information on demand. Using bundle check to do so currently leads to
highly confusing side-effects because suddenly the git working directory
might be dirty - having changed the Gemfile.lock - because bundle check has been run in the background.

This patch adds a boolean method option --[no-]lock to bundle check
that defaults to true in order to not change the current behaviour but
make it possible for users to turn it off.

Sidenote: apparently the same behaviour also applies to bundle show.
Am I the only one who finds this highly confusing?

Currently `bundle check` locks the Gemfile. According to @indirect this
behaviour is intentional (see
https://twitter.com/#!/indirect/status/186493694013210624).

I am working on a CLI monitoring tool that makes it easier to work with
cross-repository changes on projects that are split up into several
repositories.

This tool is supposed to display status information such as out-of-date
bundler Gemfiles etc. To do so it watches the filesystem and queries for
information on demand. Using `bundle check` to do so currently leads to
highly confusing side-effects because suddenly the git working directory
might be dirty - having changed the Gemfile.lock - because `bundle
check` has been run in the background.

This patch adds a boolean method option `--[no-]lock` to `bundle check`
that defaults to `true` in order to not change the current behaviour but
make it possible for users to turn it off.

Sidenote: apparently the same behaviour also applies to `bundle show`.
Am I the only one who finds this highly confusing?
@indirect
Copy link
Member

indirect commented Apr 2, 2012

I think the philosophical issue at stake here is the idea of an "out of date Gemfile". Bundler was deliberately written so that there is no such thing as an out of date Gemfile.lock during development. Loading the bundle always validates and re-resolves the Gemfile into the lockfile, by design. This makes development "just work", instead of requiring users to run 'bundle install' every time they touch their Gemfile. What kind of status are you hoping to display using your tool, and why would you want to run the entire bundler resolver without saving the output into the lockfile?

@svenfuchs
Copy link
Contributor Author

E.g. missing gems required by that repository, version conflicts, a broken gemspec, not being able to use a local override (because the repo is locked to a certain ref that is not present in the branch which is checked out locally in a repo that is being depended on), etc.

@indirect
Copy link
Member

indirect commented Apr 2, 2012

So why would you want to avoid writing out a lock file? All that does is make the next invocation take longer, since the resolver has to run again from the old starting point.

@svenfuchs
Copy link
Contributor Author

I want to avoid writing changing the lock file because this tool monitors the state of N (currently 9) local repos in the background and displays a dashboard-like overview in a terminal sidebar.

I had the situation where I was not able to switch a branch because bundle check changed the Gemfile.lock in a way so that git could not change the branch. I.e. I need a way to query information about issues that does not change anything (aka no side effects).

@svenfuchs
Copy link
Contributor Author

Message that i just ran accross after switching back to remote repos for my project (containing n related repos):

https://....git (at master) is not checked out. Please run `bundle install`

... after that monitoring tool had run bundle check and displayed the results of that. That's exactly what i want it to do. It needs to not have side effects and dirty working directories though.

@indirect
Copy link
Member

indirect commented Apr 2, 2012

You want to avoid changing the lock because you monitor multiple repos? I think I might be missing something...

Anytime check or show is run, the Gemfile must be resolved, and a "locked" resolve must be generated. I'm still having trouble figuring out why you want to generate a locked state without saving it. If a successful resolution cannot be reached, then of course no lock will be written...

@svenfuchs
Copy link
Contributor Author

Yes. I want the monitoring tool not to change the state of the repositories.

I am not sure I understand enough of Bundler's internals, but I do need to be able to check for inconsistencies of the current state of a repo without changing it in this case.

I seem to be failing at explaining this more. Not sure how to do that without repeating myself. :)

@svenfuchs
Copy link
Contributor Author

Another message I want to be able to catch easily:

The Gemfile lock is pointing to revision dd42774 but the current branch in your local override for [...] does not contain such commit. Please  make sure your branch is up to date.

@indirect
Copy link
Member

indirect commented Apr 4, 2012

So you're using local overrides? Local overrides have a special behavior that effectively means bundle update foo is run automatically for every local gem, all the time. This is deliberate, ad a way to ensure that the lockfile stays up to date with the state of the local repo. Since the local feature is brand new, maybe we can change it to fit your use case as well before we ship it.

On Apr 4, 2012, at 3:24 AM, Sven [email protected] wrote:

Another message I want to be able to catch easily:

The Gemfile lock is pointing to revision dd42774 but the current branch in your local override for [...] does not contain such commit. Please make sure your branch is up to date.


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

@svenfuchs
Copy link
Contributor Author

Are you sure they do? Was that added just recently? Because since I've added this patch (from this pull-request) to my local bundler instance I don't have that issue (with the Gemfile.lock being changed all the time) any more?

@indirect
Copy link
Member

indirect commented Apr 4, 2012

Local repos (configured via bundle config local.gemname) are designed to automatically write their HEAD sha into the lockfile, every single time the bundle is loaded. This is deliberate behaviour. We designed it that way to ensure the core Bundler promise of "you get the same code when you deploy" is honoured. For hopefully understandable reasons, I am worried about changes (like this one) that loosen that promise. It's very important to us that if bundle check gives the all clear, your lockfile will reflect the exact shas that caused the all-clear to be given. Make sense?

@svenfuchs
Copy link
Contributor Author

Ok, I'm quite close to just giving up. I might be getting everything wrong here, ...

See, I don't know a lot about bundler's internals, I just have this usecase where I want to monitor the state of my local repositories and like to make it work better (with the current unpatched bundler code it's just broken for me).

Yes, I might be one of the very first users who use the local repos feature, because that's a feature that @josevalim implemented after I've brought up the discussion again recently. We do have this issue of working with lots of interrelated local repos at a time. That's why I started working on a monitoring cli tool that gives us a better overview ... and already really proved to be very used.

If what you are saying means that bundle check always absolutely must alter the state of a repository (it would be quite badly misnamed in that case) then is there anything I can do to help find a different solution?

Or does what you say mean that bundler just won't ever provide an api for monitoring things without side effects because that's outside the scope or something?

@indirect
Copy link
Member

indirect commented Apr 4, 2012

Sorry, maybe I wasn't clear: I would like to figure out a way to make this work for you.

When we designed the current local feature, it was a very high priority to make sure that Bundler would notice changes to that local repo, and make sure that the code that shipped to production would match the code that was already there. This is a brand new (and still somewhat experimental) feature.

Your statements above make it sound like you are okay with the idea of Bundler shipping different code to production than you are currently using in development. Is that true? If so, can you explain why you're okay with that? If that's not true, is there a way that we can provide a check that you can use without having to compromise on the Bundler ideal of "the same code in production that you used in development"? Thanks!

@josevalim
Copy link
Contributor

@indirect I just think he wants a feature that checks if everything is ok without changing the Gemfile.lock. Maybe this should not be part of the bundle check command, but paths and git sources can trigger error messages, like "the repository is not checked out", "the path does not exist" and so on. So it would be useful to know about such things without necessarily changing the lock file.

He basically wants to know if bundle check will succeed or fail without any side effects. I think a better name for this option would be "--dry-run" or "--pretend".

@indirect
Copy link
Member

indirect commented Apr 5, 2012

I like --dry-run, with the implication that it will try everything but deliberately avoid side-effects, much like (for example) rsync's --dry-run option. Sven, does that seem reasonable to you?

@svenfuchs
Copy link
Contributor Author

@josevalim, thanks for helping me explain the usecase :)

@indirect, yes absolutely! (again, i personally think that check already implies no side-effects, but this now is the api and i'm not here to propose changing it, obviously)

Also, thank you for taking your time for this discussion! I'd like to apologize if I came over overly impatient above as I am of course aware that changes to Bundler's api aren't anything to do lightheartly.

Regarding your question above, i am not sure how Bundler can make sure that I am shipping the right code to production when I am working with local repositories. I might always forget to push commits and Bundler can't really know about it. Visualizing this and other potential inconsistencies/unhappy repo states is one reason why i want a monitoring tool.

But yeah, I'd be super happy with any option that allows me to "check" the Gemfile/.lock without side effects, however that option would be named :)

If you guys want me to, I'd happily change my pull request and change it to --dry-run

@travisbot
Copy link

This pull request fails (merged cc0fff1 into b3e9c1a).

@josevalim
Copy link
Contributor

👍 for dry run. @svenfuchs can you update your patch? (sorry for the late reply, feel free to ping me on gtalk after you do).

@svenfuchs
Copy link
Contributor Author

w000t, it's been merged 2ef30d6

so i'm closing my pull request :)

thanks @hone <3 <3 <3

@svenfuchs svenfuchs closed this Jul 17, 2012
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.

4 participants