This repository has been archived by the owner on Apr 14, 2021. It is now read-only.
Release 2.1.3 #7532
Merged
Release 2.1.3 #7532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7514: Fix `rake build` when path has spaces on it r=colby-swandale a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that `rake build` can no longer be run from a path that has spaces on it. ### What was your diagnosis of the problem? My diagnosis was that we need to escape the path for the shell somewhere. ### What is your fix for the problem, implemented in this PR? My fix is to add `.shellescape` at the right place. ### Why did you choose this fix out of the possible options? I chose this fix because it fixes the problem. Fixes #7512. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit 26888b6)
7515: Move `bin/rake man:check` to `linting` phase r=colby-swandale a=deivid-rodriguez So that it's only run once. ### What was the end-user problem that led to this PR? The problem was that `bin/rake man:check` is run for every CI matrix entry and that's useless. ### What was your diagnosis of the problem? My diagnosis was that we should move the check to the linting phase and run it only once, like we did for `rubocop` at #6830. ### What is your fix for the problem, implemented in this PR? My fix is to do that. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit 833faa9)
7510: Don't spawn a shell when git pushing on release r=deivid-rodriguez a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that I almost went crazy when trying to release the last bundler version. `rake release` was hanging and I didn't know why. ### What was your diagnosis of the problem? Thanks to passing `DEBUG=true` to the task, I noticed that it was `git push` and `git push --tags` commands that were hanging: ``` $ DEBUG=true bin/rake release ["gem", "build", "-V", "/home/deivid/Code/bundler/bundler.gemspec"] bundler 2.1.2 built to pkg/bundler-2.1.2.gem. ["git", "diff", "--exit-code"] ["git", "diff-index", "--quiet", "--cached", "HEAD"] ["git", "tag"] ["git", "tag", "-m", "Version 2.1.2", "v2.1.2"] Tagged v2.1.2. git push git push --tags ^Crake aborted! Interrupt: /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:198:in `read' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:198:in `popen' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:198:in `block in sh_with_status' /home/deivid/Code/bundler/lib/bundler/shared_helpers.rb:52:in `chdir' /home/deivid/Code/bundler/lib/bundler/shared_helpers.rb:52:in `block in chdir' /home/deivid/Code/bundler/lib/bundler/shared_helpers.rb:51:in `chdir' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:197:in `sh_with_status' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:133:in `perform_git_push' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:115:in `git_push' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:64:in `block (2 levels) in install' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:160:in `tag_version' /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:64:in `block in install' /home/deivid/Code/bundler/Rakefile:14:in `block in invoke' /home/deivid/Code/bundler/Rakefile:13:in `invoke' /home/deivid/Code/bundler/spec/support/rubygems_ext.rb:87:in `load' /home/deivid/Code/bundler/spec/support/rubygems_ext.rb:87:in `gem_load_and_activate' /home/deivid/Code/bundler/spec/support/rubygems_ext.rb:45:in `gem_load' Tasks: TOP => release => release:source_control_push (See full trace by running task with --trace) ``` The debugging output is very interesting because it also tells us that these commands are the only ones passing Strings to `IO.popen` instead of Arrays. That means these commands are [spawning a new shell](https://ruby-doc.org/core-2.6.5/IO.html#method-c-popen). That's when I realized that I have [hub](https://github.com/github/hub) installed on my system and `git` aliased to it. So I figure this aliasing was interacting with the subcommand in a bad way. Indeed, disabling `hub` fixed the issue and let me make the release. ### What is your fix for the problem, implemented in this PR? I think we can avoid other issues similar to this for people pushing releases by not spawing a shell here, just like we do with all of the other commands. I think it's a good practice anyways. ### Why did you choose this fix out of the possible options? I chose this fix because I think it makes the code better. Of course I can disable `hub` every time I release, but I think this is worth doing. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit b1652ef)
7520: Do `require "rubygems"` only when needed r=hsbt a=mame ### What was the end-user problem that led to this PR? The require causes circular require. See #7505 (comment) ``` $ touch empty_file $ RUBYGEMS_GEMDEPS=empty_file ./local/bin/ruby -w -e '' /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92: warning: loading in progress, circular require considered harmful - /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb from <internal:gem_prelude>:1:in `<internal:gem_prelude>' from <internal:gem_prelude>:1:in `require' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1417:in `<top (required)>' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1203:in `use_gemdeps' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/user_interaction.rb:47:in `use_ui' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems.rb:1204:in `block in use_gemdeps' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require' from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in `<top (required)>' from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler.rb:11:in `require_relative' from /home/mame/work/ruby/local/lib/ruby/2.7.0/bundler/rubygems_integration.rb:3:in `<top (required)>' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require' from /home/mame/work/ruby/local/lib/ruby/2.7.0/rubygems/core_ext/kernel_require.rb:92:in `require' ``` ### What was your diagnosis of the problem? Do not `require "rubygems"` blindly. ### What is your fix for the problem, implemented in this PR? Add `unless defined?(Gem)` to make sure it does `require "rubygems"` only when rubygems is not required. ### Why did you choose this fix out of the possible options? This is needed to release Ruby 2.7.0 so I chose the simplest fix. Co-authored-by: Yusuke Endoh <[email protected]> (cherry picked from commit 415e336)
7526: Fix CI failure with mustermann 1.1.0 r=deivid-rodriguez a=kou ### What was the end-user problem that led to this PR? This PR doesn't fix any end-user problem. This PR fixes a developer problem. Our CI is failed with mustermann 1.1.0. ### What was your diagnosis of the problem? This is caused because mustermann 1.1.0 starts depending on ruby2_keyword gem but our test script doesn't add a load path for ruby2_keyword gem yet. See also: * The PR that adds ruby2_keyword gem dependency to mustermann: sinatra/mustermann#97 * Error message on CI: #7523 (comment) ### What is your fix for the problem, implemented in this PR? My fix adds a load path for ruby2_keywrod gem installed in `tmp/1/gems/`. ### Why did you choose this fix out of the possible options? I don't have another option. Co-authored-by: Sutou Kouhei <[email protected]> (cherry picked from commit 984bef1)
7523: Fix rspec stuck problem for large stderr external command r=deivid-rodriguez a=kou
### What was the end-user problem that led to this PR?
This PR doesn't fix any end-user problem.
This PR just fixes a developer problem.
Some specs runs external commands by `sys_exec` in `spec/support/helpers.rb`. They may be stuck when they outputs many text to its stderr.
### What was your diagnosis of the problem?
`sys_exec` uses `open3`. It uses pipe to communicate an external command. If `rspec` process doesn't read stderr of the external command, the external command is stuck when the external command writes many text to its stderr. Because the external command can't write to pipe infinitely.
### What is your fix for the problem, implemented in this PR?
The current `sys_exec` tries to fix this situation but it's incomplete:
```ruby
command_execution.stdout = Thread.new { stdout.read }.value.strip
command_execution.stderr = Thread.new { stderr.read }.value.strip
```
`Thread.new { stderr.read ` isn't started until `Thread.new { stdout.read }.value` is finished. Normally, it's finished when the external command is finished. It's late. We should read stderr of the external command while the external command is alive.
### Why did you choose this fix out of the possible options?
I don't have another option.
Co-authored-by: Sutou Kouhei <[email protected]>
(cherry picked from commit 8917d9d)
7525: Use raise instead of flunk r=deivid-rodriguez a=kou ### What was the end-user problem that led to this PR? This PR doesn't fix any end-user problem. This PR fixes a developer problem. RSpec doesn't provide `flunk` method. It's a method name in test-unit and minitest. ### What was your diagnosis of the problem? 3edfddb may use `flunk` accidentally. ### What is your fix for the problem, implemented in this PR? My fix uses `raise` instead of `flunk`. ### Why did you choose this fix out of the possible options? I don't have another option. Co-authored-by: Sutou Kouhei <[email protected]> (cherry picked from commit 9b9eedd)
7519: Make `bundle config deployment true` equivalent to `bundle install --deployment` in regards to configuration r=deivid-rodriguez a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was that while addressing comments for docker-library/official-images#7188, I noticed that the alternative we're recommending for `bundle install --deployment`, i.e., `bundle config set deployment true`, is not equivalent to it. ### What was your diagnosis of the problem? My diagnosis was that whereas `bundle install --deployment` configures bundler to be frozen AND to install gems to `vendor/bundle`, `bundle config deployment true` only configures bundler to be frozen. ### What is your fix for the problem, implemented in this PR? My fix is to make `bundle config deployment true` behave just like `bundle install --deployment` in regards to configuration. ### Why did you choose this fix out of the possible options? I chose this fix because all the commands we're suggesting as alternatives for deprecations should be actual alternatives that work exactly in the same way. Also, note that there's a change scheduled for bundler 3 where the `deployment` configuration will only mean `frozen`. But for now, we should focus on making sure that people moves away from CLI flags in favor of configuration, so we might want to delay that change to make things less confusing. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit 9719efc)
7533: Rebuild man pages for January r=deivid-rodriguez a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was man pages are out of date. ### What is your fix for the problem, implemented in this PR? My fix is to bring them up to date. Co-authored-by: David Rodríguez <[email protected]> (cherry picked from commit c93740f)
deivid-rodriguez
force-pushed
the
release/2.1.3
branch
from
bf2ed1a to
784c368
Compare
|
@bundlerbot merge |
ghost
pushed a commit
that referenced
this pull request
Jan 2, 2020
7532: Release 2.1.3 r=deivid-rodriguez a=deivid-rodriguez ### What was the end-user problem that led to this PR? The problem was we should release more bug fixes. ### What is your fix for the problem, implemented in this PR? My fix is to prepare 2.1.3. Co-authored-by: Bundlerbot <[email protected]> Co-authored-by: David Rodríguez <[email protected]>
Build succeededAnd happy new year from bors! 🎉 |
ghost
merged commit
This pull request was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What was the end-user problem that led to this PR?
The problem was we should release more bug fixes.
What is your fix for the problem, implemented in this PR?
My fix is to prepare 2.1.3.