Skip to content

Check local version detection result#171

Closed
bertschneider wants to merge 1 commit intobazelbuild:masterfrom
bertschneider:fix_local_version_detection
Closed

Check local version detection result#171
bertschneider wants to merge 1 commit intobazelbuild:masterfrom
bertschneider:fix_local_version_detection

Conversation

@bertschneider
Copy link
Copy Markdown
Contributor

@bertschneider bertschneider commented Feb 12, 2024

In case the local Java version detection fails, e.g. due to a corrupt installation, skip further processing, as it results in a type error.

Closes #170

@bertschneider bertschneider requested review from a team and comius as code owners February 12, 2024 07:50
@bertschneider
Copy link
Copy Markdown
Contributor Author

As the Java installation was detected via JAVA_HOME, I'm not sure if skipping it or raising some kind of error would be the better approach.

@bertschneider
Copy link
Copy Markdown
Contributor Author

cc @fmeum

@bertschneider bertschneider force-pushed the fix_local_version_detection branch from aca5d29 to 87bdb9e Compare February 14, 2024 09:09
@bertschneider bertschneider requested a review from fmeum February 14, 2024 09:16
@fmeum
Copy link
Copy Markdown
Contributor

fmeum commented Feb 14, 2024

Looks like buildifier complains, but the other CI failure seems unrelated.

@bertschneider bertschneider force-pushed the fix_local_version_detection branch from 87bdb9e to add18e2 Compare February 15, 2024 07:28
@bertschneider bertschneider requested a review from fmeum February 15, 2024 07:29
@bertschneider
Copy link
Copy Markdown
Contributor Author

Do you have an idea what's causing the failed checks?

@fmeum
Copy link
Copy Markdown
Contributor

fmeum commented Feb 15, 2024

Do you have an idea what's causing the failed checks?

Looks like this is coming from the new test setup:

FAKE_BCR_ROOT=$(mktemp -d --tmpdir fake-bcr.XXX)
FAKE_RULES_JAVA_ROOT=${FAKE_BCR_ROOT}/modules/rules_java
FAKE_MODULE_VERSION=9999
FAKE_MODULE_ROOT=${FAKE_RULES_JAVA_ROOT}/${FAKE_MODULE_VERSION}
FAKE_ARCHIVE=${FAKE_MODULE_ROOT}/rules_java.tar.gz

cc @hvadehra

@hvadehra
Copy link
Copy Markdown
Member

Should be fixed after 88bb3ad. Could you please rebase on master?

@hvadehra hvadehra requested review from hvadehra and removed request for comius February 16, 2024 09:34
In case the local version detection fails, e.g. due to a corrupt installation,
skip further processing, as it results in a type error.
@bertschneider bertschneider force-pushed the fix_local_version_detection branch from add18e2 to 6943c45 Compare February 16, 2024 11:08
@bertschneider
Copy link
Copy Markdown
Contributor Author

bertschneider commented Feb 16, 2024

Could you please rebase on master?

Done. It fixed the failed checks, thx!

if version == None:
# Java version could not be detected
message = "Cannot detect Java version of {java_binary} in {java_home}; make sure it points to a valid Java executable"
repository_ctx.file(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It might be worth extractor a helper function to share this with the copy above, I think the only different here is the message?

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.

Broken local Java version detection

4 participants