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

Feature Idea: Create commits #49

Closed
jaredmoody opened this issue Sep 10, 2024 · 7 comments · Fixed by #53
Closed

Feature Idea: Create commits #49

jaredmoody opened this issue Sep 10, 2024 · 7 comments · Fixed by #53

Comments

@jaredmoody
Copy link

When updating multiple gems at once, it would be nice to have each update placed in an individual commit. This would allow using git bisect to find an issue in the case that an update breaks something.

I'd imagine running bundle ui --create-commits or something, select the updates, and then get a commit per update.

What do you think?

@mattbrictson
Copy link
Owner

Hi! Thanks for the suggestion. I think this could be a little tricky, because of the interdependencies between gem requirements. For example, say you are currently on rack 2.x and rails 7.0. Both of these can be updated, to rack 3.x and rails 7.2, respectively.

However, rack 3.x does not work with rails 7.0, because actionpack 7.0 has a requirement for rack ~> 2.0. Therefore, if you tried to upgrade rack and rails as separate commits, in that order, the rack upgrade wouldn't work if you tried to upgrade it first. Instead, you'd have to upgrade rails in the first commit, then rack in the second commit.

Currently, update-interactive sidesteps this problem because it updates gems all at once.

I guess we could do some sort of topological sort to determine the upgrade order? Or maybe I am overthinking it and these sorts of conflicts would not happen often enough to worry about.

One possible "brute force" approach would be this: if gems A, B, C, and D are selected to be updated, then the commits could be staged as follows:

  1. Update gem A
  2. Update gems A, B
  3. Update gems A, B, C
  4. Update gems A, B, C, D

That way:

  • If there aren't any interdependencies, then the outcome will be that each commit contains a single gem upgrade.
  • If there are interdependencies, then some commits might be empty (and thus skipped), but eventually all gems will be upgraded by the time we reach step 4.

Again, I might be making this too complicated. What do you think?

@jaredmoody
Copy link
Author

I hadn't even thought about inter-dependencies yet 🙃

Off the top of my head, I think that brute force approach would work, seems easier than creating a topology map, and likely there's not a tremendous amount of gems in an update that would make it unbearably slow.

@mattbrictson
Copy link
Owner

OK, I think this idea is worth pursuing. The way I might break it down is to make proof-of-concept:

  • Implement a simple wrapper of the Updater#apply_updates method that splits the updates into one update per gem.
  • Follow each update with a git add Gemfile Gemfile.lock && git commit -m "Update #{gem}"

If that experiment works and feels genuinely useful, then the more tedious/involved work will be to cover some of the edge cases:

  • Execute the updates accounting for gem interdependencies (e.g. using something like the brute force technique mentioned in my earlier comment)
  • Add error handling (e.g. bail out early if git doesn't work or if the Gemfile/.lock is already dirty)
  • Nicely formatted commit messages (e.g. from → to version, include link to changelog)

So overall, seems definitely doable but also a good chunk of work. My plate is a bit full at the moment so I probably won't be able to get to this right away. If you want to take a swing at it, let me know!

@mattbrictson
Copy link
Owner

Actually, the proof-of-concept was easier than expected. I pushed up a draft PR with the changes: #50

For a large number of updates, it is definitely time consuming, because Bundler has to reach out to rubygems for each update. In one of my projects, for example, it took 1.5 minutes to update 33 gems with a commit for each one. But maybe that is worth it for the git bisect value it potentially offers?

Let me know what you think.

@jaredmoody
Copy link
Author

It does take a bit, but very worth it to me - I just implemented the commit messages (PR incoming) and I'm able to use it already! 🍻

@mattbrictson
Copy link
Owner

Great! I'll try to take care of the final polish and test coverage in the coming week.

@brandondrew
Copy link

I personally would use this when I found that a commit with several gem updates caused a problem, and I wanted to break it up into separate commits automatically to make git bisect easier.

I was about to suggest an alternative that might be easier to implement, but since it looks like it's already implemented, I guess I'm late to the party and my suggestion is moot. 😅

I'm looking forward to having it available (even if I'll only pull it out after a problem is discovered 😆).

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