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

Properly handle incompatibilities on platform specific gems #6270

Merged
merged 4 commits into from
Jan 12, 2023

Conversation

deivid-rodriguez
Copy link
Member

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

When creating incompatibilities for requirements with no matching versions here:

def no_versions_incompatibility_for(package, unsatisfied_term)
cause = PubGrub::Incompatibility::NoVersions.new(unsatisfied_term)
name = package.name
constraint = unsatisfied_term.constraint
constraint_string = constraint.constraint_string
requirements = constraint_string.split(" OR ").map {|req| Gem::Requirement.new(req.split(",")) }
if name == "bundler"
custom_explanation = "the current Bundler version (#{Bundler::VERSION}) does not satisfy #{constraint}"
extended_explanation = bundler_not_found_message(requirements)
else
specs_matching_other_platforms = filter_matching_specs(@all_specs[name], requirements)
platforms_explanation = specs_matching_other_platforms.any? ? " for any resolution platforms (#{package.platforms.join(", ")})" : ""
custom_explanation = "#{constraint} could not be found in #{repository_for(package)}#{platforms_explanation}"
label = "#{name} (#{constraint_string})"
extended_explanation = other_specs_matching_message(specs_matching_other_platforms, label) if specs_matching_other_platforms.any?
end
Incompatibility.new([unsatisfied_term], :cause => cause, :custom_explanation => custom_explanation, :extended_explanation => extended_explanation)
end

We receive a pub grub version range, and need to convert it to a "standard" requirement. We parse the string representation of the version range for that. However, if the version range includes platform specific candidates, those were include the platform under parenthesis, making the string representation unparseable.

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

To fix the bug, we use only the string representation of the underlying version as the string representation of a candidate. Error messages already include the resolution platforms separately so it should not degrade with end users see or make any messages less clear.

Fixes #6267.

Make sure the following tasks are checked

These specs where nested under a completely unrelated setup.
Dealing with incompatibilities uses the string representation of
candidates, which was including the platform in parentheses, making the
version unparseable.

I think I used this string representation for debugging at some point,
but I don't think it's needed at this point, so removing it to fix the
bug.
@simi
Copy link
Member

simi commented Jan 12, 2023

ℹ️ btw. good case to utilize "hide whitespace" on "Files changed" tab for easy review.

image

@simi simi merged commit 99d51cf into master Jan 12, 2023
@simi simi deleted the fix-jruby-resolutions branch January 12, 2023 20:38
@deivid-rodriguez
Copy link
Member Author

Yay!

deivid-rodriguez pushed a commit that referenced this pull request Jan 13, 2023
Properly handle incompatibilities on platform specific gems

(cherry picked from commit 99d51cf)
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.

Gem::Requirement::BadRequirementError: Illformed requirement [" < 61.0 (java)"]
2 participants