search

zombocom / derailed_benchmarks

Go faster, off the Rails - Benchmarks for your whole Rails app
2.96k stars 144 forks source link

Failing test: Bundler re-writes gem spec when installing a git gem: #174

Closed schneems closed 1 week ago

schneems commented 4 years ago

Bundler re-writes gem spec when installing a git gem:

https://twitter.com/schneems/status/1304138043419287552

Derailed benchmarks uses git to switch between commits to benchmark code changes in a library. This was originally written to benchmark changes in Rails, but it can be used for any library.

This feature fails when using a combination of:

To recreate this failure mode I've made an intentional commit on a branch in the wicked gem:

https://github.com/zombocom/wicked/commit/10e5dd18bf8a7792b57ace0fdd27b363951b364f

What happens when the perf:library command executes is derailed will cd into the wicked directory and checkout a this commit, but this happens on top of an already modified gem spec (due to bundler).

The first checkout succeeds (because it is already HEAD)

⛄ 2.7.1 🚀  ~/.gem/ruby/2.7.1/bundler/gems/wicked-10e5dd18bf8a (main)
$ git checkout 10e5dd18bf8a7792b57ace0fdd27b363951b364f
M   wicked.gemspec
Note: switching to '10e5dd18bf8a7792b57ace0fdd27b363951b364f'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 10e5dd1 Modifying gem spec intentionally

But the second checkout fails:

$ git checkout 4b8d7f605dcecbf813226e87adac6c7d9ef22064
error: Your local changes to the following files would be overwritten by checkout:
    wicked.gemspec
Please commit your changes or stash them before you switch branches.
Aborting

Here's the full test failure.

$ BUNDLE_GEMFILE="$(pwd)/gemfiles/rails_branch.gemfile" bundle exec m test/integration/tasks_test.rb:37
config.eager_load is set to nil. Please update your config/environments/*.rb files accordingly:

  * development - set it to false
  * test - set it to false (unless you use a tool that preloads your test environment)
  * production - set it to true

/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionpack/lib/action_dispatch/middleware/stack.rb:37: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionpack/lib/action_dispatch/middleware/static.rb:111: warning: The called method `initialize' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/transactions.rb:212: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:260: warning: The called method `transaction' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:171: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:97: warning: The called method `initialize' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/persistence.rb:705: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/timestamp.rb:105: warning: The called method `_update_record' is defined here
Run options: -n "/^(test_non\\-rails_library_with_branch_specified)$/" --seed 20635

# Running:

Running: bundle info wicked --path
The dependency activerecord-jdbcsqlite3-adapter (~> 1.3.13) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for java. To add those platforms to the bundle, run `bundle lock --add-platform java`.
Running: env TEST_COUNT=10 DERAILED_SCRIPT_COUNT=2 DERAILED_PATH_TO_LIBRARY=/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/wicked-10e5dd18bf8a'
' bundle exec rake -f perf.rake perf:library --trace
** Invoke perf:library (first_time)
** Execute perf:library
Note: switching to '10e5dd18bf8a7792b57ace0fdd27b363951b364f'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at 10e5dd1 Modifying gem spec intentionally
The dependency activerecord-jdbcsqlite3-adapter (~> 1.3.13) will be unused by any of the platforms Bundler is installing for. Bundler is installing for ruby but the dependency is only for java. To add those platforms to the bundle, run `bundle lock --add-platform java`.
config.eager_load is set to nil. Please update your config/environments/*.rb files accordingly:

  * development - set it to false
  * test - set it to false (unless you use a tool that preloads your test environment)
  * production - set it to true

/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionpack/lib/action_dispatch/middleware/stack.rb:37: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionpack/lib/action_dispatch/middleware/static.rb:111: warning: The called method `initialize' is defined here
Database 'db/production.sqlite3' already exists
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/transactions.rb:212: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb:260: warning: The called method `transaction' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:171: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb:97: warning: The called method `initialize' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/persistence.rb:705: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activerecord/lib/active_record/timestamp.rb:105: warning: The called method `_update_record' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionview/lib/action_view/view_paths.rb:11: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionview/lib/action_view/lookup_context.rb:128: warning: The called method `template_exists?' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activesupport/lib/active_support/messages/rotator.rb:28: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activesupport/lib/active_support/messages/rotator.rb:6: warning: The called method `initialize' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/actionpack/lib/action_dispatch/middleware/cookies.rb:647: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activesupport/lib/active_support/message_encryptor.rb:150: warning: The called method `encrypt_and_sign' is defined here
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activesupport/lib/active_support/message_encryptor.rb:175: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/rails-3054e1d584e7/activesupport/lib/active_support/messages/metadata.rb:17: warning: The called method `wrap' is defined here
error: Your local changes to the following files would be overwritten by checkout:
    wicked.gemspec
Please commit your changes or stash them before you switch branches.
Aborting
Switched to branch 'main'
rake aborted!
Error while running "git checkout '4b8d7f605dcecbf813226e87adac6c7d9ef22064'":
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:292:in `run!'
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:58:in `block (4 levels) in <top (required)>'
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:57:in `chdir'
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:57:in `block (3 levels) in <top (required)>'
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:56:in `each'
/Users/rschneeman/Documents/projects/derailed_benchmarks/lib/derailed_benchmarks/tasks.rb:56:in `block (2 levels) in <top (required)>'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:281:in `block in execute'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:281:in `each'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:281:in `execute'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:219:in `block in invoke_with_call_chain'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:199:in `synchronize'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:199:in `invoke_with_call_chain'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/task.rb:188:in `invoke'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:160:in `invoke_task'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:116:in `block (2 levels) in top_level'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:116:in `each'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:116:in `block in top_level'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:125:in `run_with_threads'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:110:in `top_level'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:83:in `block in run'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:186:in `standard_exception_handling'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/lib/rake/application.rb:80:in `run'
/Users/rschneeman/.gem/ruby/2.7.1/gems/rake-13.0.1/exe/rake:27:in `<top (required)>'
/Users/rschneeman/.gem/ruby/2.7.1/bin/rake:23:in `load'
/Users/rschneeman/.gem/ruby/2.7.1/bin/rake:23:in `<top (required)>'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli/exec.rb:63:in `load'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli/exec.rb:63:in `kernel_load'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli/exec.rb:28:in `run'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli.rb:476:in `exec'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/command.rb:27:in `run'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/invocation.rb:127:in `invoke_command'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor.rb:399:in `dispatch'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli.rb:30:in `dispatch'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/vendor/thor/lib/thor/base.rb:476:in `start'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/cli.rb:24:in `start'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:46:in `block in <top (required)>'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/2.7.0/bundler/friendly_errors.rb:123:in `with_friendly_errors'
/Users/rschneeman/.rubies/ruby-2.7.1/lib/ruby/gems/2.7.0/gems/bundler-2.1.4/libexec/bundle:34:in `<top (required)>'
/Users/rschneeman/.gem/ruby/2.7.1/bin/bundle:23:in `load'
/Users/rschneeman/.gem/ruby/2.7.1/bin/bundle:23:in `<main>'
Tasks: TOP => perf:library
F

Failure:
TasksTest#test_non-rails_library_with_branch_specified [/Users/rschneeman/Documents/projects/derailed_benchmarks/test/integration/tasks_test.rb:31]:
Expected 'env TEST_COUNT=10 DERAILED_SCRIPT_COUNT=2 DERAILED_PATH_TO_LIBRARY=/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/wicked-10e5dd18bf8a'
' bundle exec rake -f perf.rake perf:library --trace' to return a success status.
Output: Resetting git dir of '/Users/rschneeman/.gem/ruby/2.7.1/bundler/gems/wicked-10e5dd18bf8a' to "main"

bin/rails test /Users/rschneeman/Documents/projects/derailed_benchmarks/test/integration/tasks_test.rb:37

Finished in 4.183385s, 0.2390 runs/s, 0.2390 assertions/s.
1 runs, 1 assertions, 1 failures, 0 errors, 0 skips
schneems commented 4 years ago

Here's a diff of what bundler does to the gemspec: https://gist.github.com/schneems/74016ca172baea81efe88d85f4cb42c5

If you try to stash this change then Bundler::Source::Git will error because it assumes that all gemspecs have this "stub" line. Here's a backtrace: https://gist.github.com/schneems/d169d045731ac15cb0988985a659f5fc

You can work around this in the short term by putting this in your Gemfile:

class Bundler::Source::Git
  def load_gemspec(file)
    super
  end
end

This works because this class inherits from Bundler::Source::Path. I spoke with bundler core and