-
Notifications
You must be signed in to change notification settings - Fork 377
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
Add GemfileLock updater to bump version in Gemfile.lock for Ruby #1768
Comments
I'm having this same problem with every Github release action I've tried. @andrewthauer Perhaps a simpler and more general approach for release procedures which have to run commands is to allow any commands to be run just before the Release Please changes are committed? For example...
|
@schwern - Thanks! Yes a hook before commit would also work. I had to double check, but I didn't see any option that currently does this. I'm not sure how I would go about adding this atm though. I can dig into the code more unless you can point me in the right direction? |
@andrewthauer Sorry, I haven't looked at the code. It's just a thought I had. Hooks would be very useful. In addition to before commit, after commit but before push would also be useful. It could run the code in the Release Please branch with Git configured so one could, for example, update the Gemfile.lock in its own commit.
That would be nice because it would separate the Release Please work from the |
@andrewthauer Do you have a work around for the Gemfile.lock issue that doesn't involve patching Release Please? |
No, I do not unfortunately. Which is why I attempted to create a fix. |
@andrewthauer I think I have a work around. https://github.com/momentohq/client-sdk-ruby/blob/main/.github/workflows/on-pr-to-main-update-gemfile-lock.yml and you can see it in action here momentohq/client-sdk-ruby#108 This action will fire whenever a PR is opened that changes the Gemfile, version.rb, or gemspec and does not change the Gemfile.lock. It updates the Gemfile.lock and commits. This seems like generally a Good Thing to do for PRs. For more specificity, you can make the action only trigger when a PR is opened with the The gotcha is that if you run a workflow with the normal GITHUB_TOKEN it will not trigger additional workflows. So the checks will not be re-run on the PR. This is to avoid loops. So you have to use a different token in the action. |
Interesting, but wouldn't the I like the idea of the hook as a generic solution, but I think my patch would also work and shouldn't cause any issues as the updater will ignore files that do not exist. Another alternative, seemed to be a plugin, which seem to run after all packages are updated, but before the commits. However, there didn't seem to be much documentation on writing plugins or testing them that I saw. |
It should be pretty trivial to add an updater that knows how to update a Gemfile.lock file. You could then have the ruby strategy attempt to update that file only if it exists. We would be happy to take that feature if you can help implement, or we will get to it when we have time. |
We don't run e2e tests as part of CI, we test the unit test each component and have some compatibility tests for complicated configs. If you want to test your fork against GitHub, you can compile the CLI in your branch and run it against your repo: https://github.com/googleapis/release-please/blob/main/CONTRIBUTING.md#testing-a-new-feature-using-cli |
@andrewthauer Have another look at the PR. The change to Gemfile.lock is made in the PR branch. Release Please tags and releases after the PR is merged, so the tag and release will include the Gemfile.lock. I haven't made a release yet to fully test this, but it should work.
I'm a little skeptical about updating the Gemfile.lock with a regex, but it is a good approximation. Thanks for taking this on, the lack of bundler support in these semantic release actions has been driving me crazy for a month. 👍 |
@schwern - I'll have another try at your workaround. You solution initially wouldn't work because I'm actually triggering a release after every merge to the main branch via some automation using the @chingor13 - Thanks for the information. Even if I can get a workaround working, I think having this as a native feature of |
@andrewthauer I realized that since I'm developing a gem, I probably shouldn't commit the Gemfile.lock. 🙄 I know this is against Bundler recommendations, but gem users will have all sorts of different dependencies and I want to reflect that in dev and testing. So that might make my release problems go away. |
Actually, it used to be the recommendation to not commit the lockfile but not anymore. See https://bundler.io/guides/faq.html#using-gemfiles-inside-gems (might need to search). We changed over to this, because CI would fail for any gems that are not updated frequently because it pulls in the latest versions. We now commit lock files for gems and have GH create dependency PRs to ensure they are up to date and/or require fixes due to version bumps. |
@andrewthauer My thought is that neither using a locked set of dependencies, nor installing the latest deps is correct. Users can have any valid set of dependencies installed. I've been looking into simulating this by installing random, but valid, versions of dependencies in CI, in addition to running with the latest. https://stackoverflow.com/questions/74776586/can-i-install-random-but-valid-dependencies-of-a-gem For example, I'm pretty sure this gemspec is incorrect. I doubt it will work on grpc 1.0.0. I could manually search for the min dependency, but I'd rather have CI test for it. |
@chingor13 - I've opened PR #1790 to add support for a Question. What is the purpose of the |
Is your feature request related to a problem? Please describe.
I'm trying to use
release-please
with Ruby gems that haveGemfile.lock
committed. The newest official recommendation for this is to commit the lock file (see Should I commit myGemfile.lock
when writing a gem? in the officialbundler
FAQ). Typically you runbundle install
after aversion.rb
bump which updates a line at the top of the lock file indicating the new version. However, thisrelease-please
currently does not do this so CI steps can fail to proceed as the lock file is out of date.Describe the solution you'd like
I'd like to add a new
GemfileLock
updater to bump the Gemfile.lock appropriately similar to other lock file updaters in other strategies.Describe alternatives you've considered
I looked at using the
extra-files
flag, but since the ruby gem lock file is not a standard format and can't be a template this doesn't seem to be an option.Additional context
I've started a fork / branch with what I think might be the appropriate changes. I'm not exactly sure the best way to do a full e2e test atm yet though.
The text was updated successfully, but these errors were encountered: