Skip to content

Conversation

@aibaars
Copy link
Contributor

@aibaars aibaars commented Jun 17, 2020

The extendedJavaHome environment variable contains . symbols in the version. This causes the environment variable to be ignored by the action runner. This makes the variable inaccessible with bash's ${VAR} syntax . It's best to replace those and other non standard symbols with and underscore.

For reference:

Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set .

See also: https://unix.stackexchange.com/questions/93532/exporting-a-variable-with-dot-in-it

The extendedJavaHome environment variable contains `.` symbols in the version. This causes the environment variable to be ignored by the action runner. It's best to replace those and other non standard symbols with and underscore.

For reference:
> Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set .
@aibaars aibaars marked this pull request as ready for review June 17, 2020 16:24
@konradpabjan konradpabjan requested a review from ericsciple June 17, 2020 17:10
// uppercase letters, digits, and the underscore. Therefore we convert
// the extendedJavaHome variable to upper case and replace '.' symbols and
// any other non-alphanumeric characters with an underscore.
extendedJavaHome = extendedJavaHome.toUpperCase().replace(/[^0-9A-Z_]/g, '_');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a breaking change. Should the old variable be set also, so existing customers dont get broken?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. It could break certain users if they're actually able to use the current variable.

When we eventually release a v2 version of setup-java, then we can remove the old one (we don't promise compat between the new versions).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, I just pushed a new commit that keeps the old variable.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants