-
Notifications
You must be signed in to change notification settings - Fork 23
fix: make sure 'bundle install' is always called during updates #420
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
base: master
Are you sure you want to change the base?
Conversation
|
@doudou I'm also intrigued by run_package_managers_without_packages. As far as I can see, resolve_and_partition_osdep_packages will never return an empty list unless the manager explicitly requires it (by implementing Manager#call_while_empty?), in which case |
0301754 to
92e8a32
Compare
|
@doudou Ping |
| options = [] | ||
| binstubs_path = File.join(root_dir, "bin") | ||
| if updated || !install_only || !File.file?("#{gemfile_path}.lock") | ||
| if updated || !File.file?("#{gemfile_path}.lock") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this one
"install_only" means "do not update". We are already checking if the gemfile got updated and run unconditionally, I don't understand why !install_only was removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... I'm gonna have to go back to it. It's been a while (should have put a comment)
| def install( | ||
| osdep_packages, all: nil, install_only: false, | ||
| run_package_managers_without_packages: false, **options | ||
| run_package_managers_without_packages: true, **options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. It feels like this is duplicated with the call_while_empty? flag. I'm guessing it was two attempts at fixing the same issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wanna remove it or keep it for backwards compatibility (although this is a "very" private API...)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with removing it.
To ensure that a lock file always exists and is up-to-date in the prefix,
BundlerManager#installmust be called during all update operations.Fixes #419