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

Normalize the version before it is looked up in the tool cache #56

Open
wants to merge 2 commits into
base: master
from

Conversation

@plamentotev
Copy link

plamentotev commented Apr 14, 2020

Currently the resolved version is cached. For example something like 8.0.141. On other hand when a version is looked up in the cache the string provided by the user is used. This may lead to unexpected behavior. 1.8 will not match 8.0.141. Normalizing the version string before looking up in the tool cache will solve the problem. The normalized version (8.x) will match the entry in the cache as expected.

Normalizing the string in the beginning of the flow won't hurt as it is a valid version specification. What is more it the user can supply the normalized version directly so the code should be able to work with it.

Fixes #54

plamentotev added 2 commits Apr 14, 2020
Currently the resolved version is cached. For example something like 8.0.141.
On other hand when a version is looked up in the cache the string provided
by the user is used. This may lead to unexpected behavior. 1.8 will not match
8.0.141. Normalizing the version string before looking up in the tool cache
will solve the problem. The normalized version (8.x) will match the
entry in the cache as expected.

Normalizing the string in the beginning of the flow won't hurt
as it is a valid version specification. What is more it the user can supply
the normalized version directly so the code should be able to work with it.
@plamentotev
Copy link
Author

plamentotev commented Apr 14, 2020

Judging by the failed test it is not the resolved version that is being stored in the cache but the version spec (the version string provided by the user.)

EDIT: Only when JDK file is provided.

@plamentotev
Copy link
Author

plamentotev commented Apr 14, 2020

Hmm, I can update my change so that the version is normalized only if jdkFile is not provided. Then it will work.

Or at least kind of. It will work if the user entered version like 12 and not 12.x On one hand it makes sense - why would you enter version range if you supplied the file. But on other hand does it makes sense to use version and tool cache if you have supplied the jdk file? What would be the use case? To populate the cache for other jobs that does not provide jdkFile? Because if I provide the file then does it matter what version I provide and why would I like to take it from the cache?

@ZacSweers
Copy link

ZacSweers commented Apr 30, 2020

Is this PR being looked by anyone at github?

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

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.