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

Allow rake release to ask for input (3rd take) #7199

Merged
3 commits merged into from
Jun 12, 2019
Merged

Allow rake release to ask for input (3rd take) #7199

3 commits merged into from
Jun 12, 2019

Conversation

deivid-rodriguez
Copy link
Member

This PR supersedes #7108 and #7005.

It fixes #6854.

What was the end-user problem that led to this PR?

The problem was that if users has 2FA authentication on their rubygems account, rake release doesn't really work at the moment, since it hangs asking for the OTP code, but without letting the user know.

What was your diagnosis of the problem?

My diagnosis was that we need to allow the rake release helper that shells out to gem push to read gem push output and show it to the user, so that she can introduce the requested information.

What is your fix for the problem, implemented in this PR?

My fix is inspired by @segiddins's comment in #6854 (comment). Kernel#system works like we would expect in this situation.

Why did you choose this fix out of the possible options?

I chose this fix because #7108 had a few problems:

  • It would update the sh helper, which is used in many different places. This was unnecessary since most of the times we shell out to the gem CLI we don't need to ask for input, and it also produced a very verbose output in those cases, since everything the gem CLI prints to the screen would be printed by the bundler helpers too. This PR does not change the current output, other than for rake release.

  • It would print duplicate output. This is a rake release test using Fix GemHelper to allow reading Input/Output #7108:

    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
      Successfully built RubyGem
      Name: rake_release_tester
      Version: 0.1.0
      File: rake_release_tester-0.1.0.gem
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    v0.1.0
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asd
    Your OTP code is incorrect. Please check it and retry.
    rake aborted!
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   Your OTP code is incorrect. Please check it and retry.
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:187:in `sh'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:109:in `rubygem_push'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:70:in `block in install'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `kernel_load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:28:in `run'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:468:in `exec'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:26:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:17:in `start'
    ../bundler/exe/bundle:30:in `block in <main>'
    /home/deivid/Code/bundler/lib/bundler/friendly_errors.rb:123:in `with_friendly_errors'
    ../bundler/exe/bundle:22:in `<main>'
    Tasks: TOP => release => release:rubygem_push
    (See full trace by running task with --trace)
    

    This is the same test using this PR:

    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asdfasdf
    Your OTP code is incorrect. Please check it and retry.
    
  • Previous approach was hard to test. The test included here might not be great but it's something...

deivid-rodriguez and others added 3 commits June 11, 2019 11:44
So we can later add more tests with the same setup.
Co-authored-by: Colby Swandale <me@colby.fyi>
Co-authored-by: Kevin Deisz <kevin.deisz@gmail.com>
@colby-swandale
Copy link
Member

@bundlerbot r+

ghost pushed a commit that referenced this pull request Jun 12, 2019
7199: Allow `rake release` to ask for input (3rd take) r=colby-swandale a=deivid-rodriguez

This PR supersedes #7108 and #7005.

It fixes #6854.

### What was the end-user problem that led to this PR?

The problem was that if users has 2FA authentication on their rubygems account, `rake release` doesn't really work at the moment, since it hangs asking for the OTP code, but without letting the user know.

### What was your diagnosis of the problem?

My diagnosis was that we need to allow the `rake release` helper that shells out to `gem push` to read `gem push` output and show it to the user, so that she can introduce the requested information.

### What is your fix for the problem, implemented in this PR?

My fix is inspired by @segiddins's comment in #6854 (comment). `Kernel#system` works like we would expect in this situation. 

### Why did you choose this fix out of the possible options?

I chose this fix because #7108 had a few problems:

* It would update the `sh` helper, which is used in many different places. This was unnecessary since most of the times we shell out to the `gem` CLI we don't need to ask for input, and it also produced a very verbose output in those cases, since everything the `gem` CLI prints to the screen would be printed by the bundler helpers too. This PR does not change the current output, other than for `rake release`.

* It would print _duplicate_ output. This is a `rake release` test using #7108:

    ```
    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
      Successfully built RubyGem
      Name: rake_release_tester
      Version: 0.1.0
      File: rake_release_tester-0.1.0.gem
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    v0.1.0
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asd
    Your OTP code is incorrect. Please check it and retry.
    rake aborted!
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   Your OTP code is incorrect. Please check it and retry.
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:187:in `sh'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:109:in `rubygem_push'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:70:in `block in install'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `kernel_load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:28:in `run'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:468:in `exec'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:26:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:17:in `start'
    ../bundler/exe/bundle:30:in `block in <main>'
    /home/deivid/Code/bundler/lib/bundler/friendly_errors.rb:123:in `with_friendly_errors'
    ../bundler/exe/bundle:22:in `<main>'
    Tasks: TOP => release => release:rubygem_push
    (See full trace by running task with --trace)
    ```

    This is the same test using this PR:

    ```
    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asdfasdf
    Your OTP code is incorrect. Please check it and retry.
    ```

* Previous approach was hard to test. The test included here might not be great but it's something...

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
@ghost
Copy link

ghost commented Jun 12, 2019

Build succeeded

@ghost ghost merged commit 1b2bbc7 into master Jun 12, 2019
@ghost ghost deleted the deivid/allow_otp branch June 12, 2019 08:42
colby-swandale pushed a commit that referenced this pull request Jun 12, 2019
7199: Allow `rake release` to ask for input (3rd take) r=colby-swandale a=deivid-rodriguez

This PR supersedes #7108 and #7005.

It fixes #6854.

### What was the end-user problem that led to this PR?

The problem was that if users has 2FA authentication on their rubygems account, `rake release` doesn't really work at the moment, since it hangs asking for the OTP code, but without letting the user know.

### What was your diagnosis of the problem?

My diagnosis was that we need to allow the `rake release` helper that shells out to `gem push` to read `gem push` output and show it to the user, so that she can introduce the requested information.

### What is your fix for the problem, implemented in this PR?

My fix is inspired by @segiddins's comment in #6854 (comment). `Kernel#system` works like we would expect in this situation.

### Why did you choose this fix out of the possible options?

I chose this fix because #7108 had a few problems:

* It would update the `sh` helper, which is used in many different places. This was unnecessary since most of the times we shell out to the `gem` CLI we don't need to ask for input, and it also produced a very verbose output in those cases, since everything the `gem` CLI prints to the screen would be printed by the bundler helpers too. This PR does not change the current output, other than for `rake release`.

* It would print _duplicate_ output. This is a `rake release` test using #7108:

    ```
    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
      Successfully built RubyGem
      Name: rake_release_tester
      Version: 0.1.0
      File: rake_release_tester-0.1.0.gem
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    v0.1.0
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asd
    Your OTP code is incorrect. Please check it and retry.
    rake aborted!
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   Your OTP code is incorrect. Please check it and retry.
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:187:in `sh'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:109:in `rubygem_push'
    /home/deivid/Code/bundler/lib/bundler/gem_helper.rb:70:in `block in install'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:69:in `kernel_load'
    /home/deivid/Code/bundler/lib/bundler/cli/exec.rb:28:in `run'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:468:in `exec'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/invocation.rb:126:in `invoke_command'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor.rb:387:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:26:in `dispatch'
    /home/deivid/Code/bundler/lib/bundler/vendor/thor/lib/thor/base.rb:466:in `start'
    /home/deivid/Code/bundler/lib/bundler/cli.rb:17:in `start'
    ../bundler/exe/bundle:30:in `block in <main>'
    /home/deivid/Code/bundler/lib/bundler/friendly_errors.rb:123:in `with_friendly_errors'
    ../bundler/exe/bundle:22:in `<main>'
    Tasks: TOP => release => release:rubygem_push
    (See full trace by running task with --trace)
    ```

    This is the same test using this PR:

    ```
    $ RUBYOPT="-I../bundler/lib" ../bundler/exe/bundle exec rake release
    rake_release_tester 0.1.0 built to pkg/rake_release_tester-0.1.0.gem.
    Tag v0.1.0 has already been created.
    Pushing gem to https://rubygems.org...
    You have enabled multi-factor authentication. Please enter OTP code.
    Code:   asdfasdf
    Your OTP code is incorrect. Please check it and retry.
    ```

* Previous approach was hard to test. The test included here might not be great but it's something...

Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>
(cherry picked from commit cd05f13)
colby-swandale added a commit that referenced this pull request Jun 13, 2019
* 2-0-stable: (89 commits)
  fix changelog 2.0.2 typos
  add v2.0.2 changelog
  bump version to 2.0.2
  Merge #7199
  fix bug where bundler v3 is running a test for bundflet 2
  Merge #6798
  add bors configuation
  port GemHelper from master
  Merge #7080
  Merge #7089
  Merge #7068
  Merge #7036
  Merge #7067
  change Bundler 3 specs in travis to use RubyGems 3.0.3
  bump RubyGems v3 to the latest version on Travis
  Merge #6963
  Merge #7078
  Merge pull request #7061 from bundler/fix_circular_requires
  Merge #6864
  remove linting step in travis (it will still run in each build)
  ...
@hsbt hsbt modified the milestones: 2.0.2, 2.0.3 Aug 5, 2019
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Sep 18, 2019
## 2.0.2 (2019-05-13)

Changes:

  - Fixes for Bundler integration with ruby-src ([#6941](rubygems/bundler#6941), [#6973](rubygems/bundler#6973), [#6977](rubygems/bundler#6977), [#6315](rubygems/bundler#6315), [#7061](rubygems/bundler#7061))
  - Use `__dir__` instead of `__FILE__` when generating a gem with `bundle gem` ([#6503](rubygems/bundler#6503))
  - Use `https` on externals links in the Bundler gemspec ([#6721](rubygems/bundler#6721))
  - Removed duplicate gem names from the suggested `did you mean` list for gem typos ([#6739](rubygems/bundler#6739))
  - Removed Ruby 1.x compatibility code ([#6764](rubygems/bundler#6764), [#6806](rubygems/bundler#6806))
  - Fixed an issue where `bundle remove` would crash with certain Gemfiles ([#6768](rubygems/bundler#6769))
  - Fixed indentation in the Bundler executable template ([#6773](rubygems/bundler#6773))
  - Fixed an issue where plugins could register for the same Bundler hook multiple times ([#6775](rubygems/bundler#6775))
  - Changed the "multiple sources" message in `bundle install` to be a warning instead of an error ([#6790](rubygems/bundler#6790))
  - Fixed a bug where path gems would break when using `only_update_to_newer_versions` ([#6774](rubygems/bundler#6774))
  - Fixed a bug where installing plugins with the `--delpoyment` setting would fail ([#6805](rubygems/bundler#6805))
  - Fixed an issue where `bundle update` couldn't update & install a gem when `no_install` was set (a `bundle package` config) ([#7078](rubygems/bundler#7078))
  - Fixed an issue where users could not run `bundle exec` on default gems ([#6963](rubygems/bundler#6963))
  - Updated vendor libraries to their latest version ([#7076](rubygems/bundler#7067), [#7068](rubygems/bundler#7068))
  - Fixed an issue where the `github` source was not using `https` by default that we mentioned in the 2.0 release ([#7182](rubygems/bundler#7182))
  - Fixed an issue where `rake release` was not outputting the message to users asking for a 2fa token ([#7199](rubygems/bundler#7199))

Documentation:

  - Fix incorrect documented `BUNDLE_PATH_RELATIVE_TO_CWD` env var ([#6751](rubygems/bundler#6751))
  - Update URLs in Bundler's documentation to use `https` ([#6935](rubygems/bundler#6935))

## 2.0.1 (2019-01-04)

Changes:

  - Relaxed RubyGems requirement to `>= 2.5.0` ([#6867](rubygems/bundler#6867))

## 2.0.0 (2019-01-03)

No new changes

## 2.0.0.pre.3 (2018-12-30)

Breaking Changes:

  - Bundler 2 now requires RubyGems 3.0.0 at minimum

Changes:

  - Ruby 2.6 compatibility fixes (@segiddins)
  - Import changes from Bundler 1.17.3 release

Note: To upgrade your Gemfile to Bundler 2 you will need to run `bundle update --bundler`

## 2.0.0.pre.2 (2018-11-27)

Breaking Changes:

  - `:github` source in the Gemfile now defaults to using HTTPS

Changes

  - Add compatibility for Bundler merge into ruby-src

Note: To upgrade your Gemfile to Bundler 2 you will need to run `bundle update --bundler`

## 2.0.0.pre.1 (2018-11-09)

Breaking Changes:

  - Dropped support for versions of Ruby < 2.3
  - Dropped support for version of RubyGems < 2.5
  - Moved error messages from STDOUT to STDERR

Note: To upgrade your Gemfile to Bundler 2 you will need to run `bundle update --bundler`
@ioquatix
Copy link

I recently tried this and it worked. There was one issue.

I had bundler 2.0.2 installed, but because of the Gemfile.lock it was still trying to use 1.17.something - deleting the Gemfile.lock fixed this issue and allowed me to use 2.0.2 which has the OTP support.

Great work sorting this out! Very useful.

@indirect
Copy link
Member

@ioquatix fyi in the future, you can also run bundle update --bundler to keep your Gemfile but also use the latest bundler. We're hoping to not require a manual step in the future 👍

This pull request was closed.
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.

Enabling TOTP on rubygems.org breaks rake release
5 participants