Skip to content

Conversation

@deivid-rodriguez
Copy link
Contributor

@deivid-rodriguez deivid-rodriguez commented Oct 10, 2022

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

I did some cleanups recently to avoid unnecessary sorting and speed things up at #5868. But one change in particular caused issues on Windows.

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

I identified this code here: https://github.com/rubygems/rubygems/blob/629e08d34d14f60fe3398e40fc72f4f171ffc582/bundler/lib/bundler/lazy_specification.rb#L82-L84 was relying on the index search returning a particular order.

I think we'll probably need to revert that commit but I also found that this code should not be being hit by default on Windows, since it's only a fallback for backwards compatibility with lockfiles using ruby or java as their main platform, and that's not the case on Windows.

This commit fixes that latter issue and adds a realworld CI test that installs Rails and generates a dummy application on Windows, to hopefully avoid this kind of widespread issues on Windows in the future.

Fixes #5970.

Make sure the following tasks are checked

I didn't realize how the `Bundler::GemHelpers.generic` method works when
I added this. It already matches this and other java platforms properly.
It was using the originally given platform. It's the same as the real
gemspec platform, but in some edge cases it could be different.
@joshcooper
Copy link

Thank you for fixing this @deivid-rodriguez Do you know when this might get merged and released? Trying to decide whether to wait or pin back to an older version.

@deivid-rodriguez
Copy link
Contributor Author

Normally I would release new versions next Wednesday, but this particular bug has broken very basic usages on Windows, so I'm considering an earlier release. I can't promise anything though, so if next Wednesday is too late for you I suggest pinning.

@deivid-rodriguez deivid-rodriguez merged commit 2428fb6 into master Oct 13, 2022
@deivid-rodriguez deivid-rodriguez deleted the windows-fix branch October 13, 2022 04:46
@carnoxen

This comment was marked as resolved.

deivid-rodriguez added a commit that referenced this pull request Oct 17, 2022
Fix incorrect materialization on Windows

(cherry picked from commit 2428fb6)
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.

NoMethodError: undefined method `full_name' for nil:NilClass ! Rails 7

4 participants