Skip to content
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

Fix gem pristine sometimes failing to pristine user installed gems #7664

Merged

Conversation

deivid-rodriguez
Copy link
Member

@deivid-rodriguez deivid-rodriguez commented May 16, 2024

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

If the same version of a gem is installed in the default gem home, and in the user home, gem pristine will fail to pristine the user installed one.

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

Make sure all copies are properly pristined by not removing duplicates from the stubs list.

Make sure the following tasks are checked

@deivid-rodriguez deivid-rodriguez marked this pull request as ready for review May 16, 2024 16:17
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/gem-pristine-user-installed branch from ad58e96 to 0eb6ed8 Compare May 29, 2024 14:20
@deivid-rodriguez deivid-rodriguez merged commit b678d92 into master May 29, 2024
75 checks passed
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/gem-pristine-user-installed branch May 29, 2024 15:35
@k0kubun
Copy link
Contributor

k0kubun commented May 29, 2024

@deivid-rodriguez ruby/ruby MinGW and Visual Studio jobs started to fail after this is synchronized to ruby/ruby. I suspect there's a Windows-related issue. Could you take a look?

https://github.com/ruby/ruby/actions/runs/9289018414/job/25561871390
https://github.com/ruby/ruby/actions/runs/9289018425/job/25561873060

@deivid-rodriguez
Copy link
Member Author

Sorry about that @k0kubun, and thanks for the quick ping. I think I know the reason, feel free to skip those or revert the commit and I'll let you know when I think it's fixed!

@MSP-Greg
Copy link
Contributor

The same issue in ruby-loco.

I think I know the reason

Ok, I'll wait for you.

BTW, when I saw this, I looked at some of the code. There are many places in the code where File.exist? is used for a directory check. GH search showed a lot of files with File.exist?. Wondered about a PR, but I haven't been in the code for a while...

@deivid-rodriguez
Copy link
Member Author

I think basically the list of specifications can now have duplicate specifications with same name, versions and platform (but different install location), but sorting has not been adapted to make sure it's still stable after this.

@MSP-Greg
Copy link
Contributor

@deivid-rodriguez

Not sure if you've had time to look at this. Windows CI isn't running.

See:
https://github.com/rubygems/rubygems/actions/runs/9036340066/job/24833029608 for PR #7648.

bin/rake won't run in a normal Windows shell. If you want cross-platfrom command, maybe ruby bin/rake <arg?>?

@MSP-Greg
Copy link
Contributor

MSP-Greg commented May 29, 2024

I tried the above, see https://github.com/MSP-Greg/rubygems/actions/runs/9294061049.

Note that Windows Ruby 3.0 had a problem with the "Install Dependencies / ruby bin/rake setup" step, all other Windows jobs were fine... The tests ran, but they failed.

EDIT: Wondering why this doesn't fail. I checked 'Windows Terminal', and running bin/rake in it opens a GUI window asking what program/executable should be used to open the file. Using VSC, it works as in CI; the command does nothing.

@hsbt
Copy link
Member

hsbt commented May 30, 2024

@deivid-rodriguez I reverted this at ruby/ruby@ba8e6e7.

@deivid-rodriguez
Copy link
Member Author

Thanks @hsbt, I will look into this today.

@deivid-rodriguez
Copy link
Member Author

Do the jobs that fail run on a regular PR to ruby/ruby, or only after merge?

@hsbt
Copy link
Member

hsbt commented May 30, 2024

It's happened with after merging. And I easily reproduce that like the followings:

Retrying...
[  7/186] TestGem#test_gem_path_ordering = 0.51 s
  1) Failure:
TestGem#test_gem_path_ordering [C:/Users/hsbt/source/repos/ruby/ruby/test/rubygems/test_gem.rb:1584]:
Wrong spec before require for dir0.
<"C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-fqoh63/gemhome/gems/m-1"> expected but was
<"C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-fqoh63/userhome/.local/share/gem/ruby/3.4.0+0/gems/m-1">.

[ 24/186] TestGem#test_gem_path_ordering_short = 0.49 s
  2) Failure:
TestGem#test_gem_path_ordering_short [C:/Users/hsbt/source/repos/ruby/ruby/test/rubygems/test_gem.rb:1619]:
Wrong spec selected.
<"C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-7ootz/gemhome/gems/m-1"> expected but was
<"C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-7ootz/userhome/.local/share/gem/ruby/3.4.0+0/gems/m-1">.

[135/186] TestGemCommandsPristineCommand#test_execute_missing_cache_gem_when_multi_repo = 0.76 s
  3) Failure:
TestGemCommandsPristineCommand#test_execute_missing_cache_gem_when_multi_repo [C:/Users/hsbt/source/repos/ruby/ruby/test/rubygems/test_gem_commands_pristine_command.rb:485]:
<"Restored b-1 in C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-f8psab/gemhome"> expected but was
<"Cached gem for b-1 not found, attempting to fetch...">.

[185/186] TestGemCommandsUpdateCommand#test_execute_system_update_installed_in_non_default_gem_path = 0.72 s
  4) Error:
TestGemCommandsUpdateCommand#test_execute_system_update_installed_in_non_default_gem_path:
Errno::ENOENT: No such file or directory @ dir_chdir0 - C:/Users/hsbt/AppData/Local/Temp/rubytest.vodqpy/test_rubygems_20240530-14996-e6utt7/gemhome/gems/rubygems-update-9
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/commands/update_command.rb:182:in 'Dir.chdir'
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/commands/update_command.rb:182:in 'Gem::Commands::UpdateCommand#install_rubygems'
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/commands/update_command.rb:287:in 'Gem::Commands::UpdateCommand#update_rubygems'
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/commands/update_command.rb:99:in 'Gem::Commands::UpdateCommand#execute'
    C:/Users/hsbt/source/repos/ruby/ruby/test/rubygems/test_gem_commands_update_command.rb:240:in 'block in TestGemCommandsUpdateCommand#test_execute_system_update_installed_in_non_default_gem_path'
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/user_interaction.rb:46:in 'Gem::DefaultUserInteraction.use_ui'
    C:/Users/hsbt/source/repos/ruby/ruby/lib/rubygems/user_interaction.rb:69:in 'Gem::DefaultUserInteraction#use_ui'
    C:/Users/hsbt/source/repos/ruby/ruby/test/rubygems/test_gem_commands_update_command.rb:239:in 'TestGemCommandsUpdateCommand#test_execute_system_update_installed_in_non_default_gem_path'

Finished tests in 827.368433s, 31.3838 tests/s, 7567.0037 assertions/s.
25966 tests, 6260700 assertions, 3 failures, 1 errors, 539 skips

ruby -v: ruby 3.4.0dev (2024-05-29T19:58:35Z master 4a9ef9e23c) [x64-mswin64_140]
NMAKE : fatal error U1077: '@ .\ruby.exe -I../../repos/ruby/ruby/lib -I".ext/x64-mswin64_140" -I. "../../repos/ruby/ruby/tool/runruby.rb" --extout=".ext"  -- --disable-gems -r../../repos/ruby/ruby/tool/lib/_tmpdir  "../../repos/ruby/ruby/test/runner.rb" --ruby=".\ruby.exe -I../../repos/ruby/ruby/lib -I".ext/x64-mswin64_140" -I. "../../repos/ruby/ruby/tool/runruby.rb" --extout=".ext"  -- --disable-gems"  --excludes-dir=../../repos/ruby/ruby/test/.excludes --name=!/memory_leak/  -j4' : return code '0x4'
Stop.

Please let me know If you want to try your fix with my environment.

@deivid-rodriguez
Copy link
Member Author

Cool, I'll let you know once ready so you can try locally. It'd be good to add this environment to our CI if possible too.

@deivid-rodriguez
Copy link
Member Author

So thanks to @MSP-Greg we uncovered why this issue went unnoticed, and #7700 should fix things!

@deivid-rodriguez
Copy link
Member Author

@hsbt We've now fixed this so RubyGems can be resynced in ruby/ruby again.

@hsbt
Copy link
Member

hsbt commented May 31, 2024

Great! I will merge them in the next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants