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

Exit from setupAntEnv is not propagated to its only usecase #3839

Merged
merged 8 commits into from
Jun 20, 2024

Conversation

judovana
Copy link
Contributor

@judovana judovana commented Jun 5, 2024

I had hit this when I forget to run all the build chain with JAVA_HOME set. It died, but without any trace. This PR is adding the trace of what went wrong.

@judovana
Copy link
Contributor Author

judovana commented Jun 5, 2024

I do not have preffernce whether it is 895dd6a or 8b866a5.

I like 895dd6a more, but it may be to vague.

sbin/build.sh Outdated Show resolved Hide resolved
@judovana judovana requested a review from karianna June 10, 2024 14:58
@judovana
Copy link
Contributor Author

spellcheck should have warned to this: https://www.shellcheck.net/wiki/SC2155#problematic-code-in-the-case-of-local

@judovana
Copy link
Contributor Author

Hard to say where it is intentional:

$  grep -ir  SC2155
build-farm/platform-specific-configurations/mac.sh:# shellcheck disable=SC2155
build-farm/platform-specific-configurations/alpine-linux.sh:# shellcheck disable=SC2155
build-farm/platform-specific-configurations/aix.sh:# shellcheck disable=SC2155
build-farm/platform-specific-configurations/windows.sh:# shellcheck disable=SC2155
build-farm/platform-specific-configurations/linux.sh:# shellcheck disable=SC2155
sbin/prepareWorkspace.sh:# shellcheck disable=SC2155,SC1091,SC2196,SC2235
sbin/common/config_init.sh:# shellcheck disable=SC2153,SC2155
sbin/common/common.sh:# shellcheck disable=SC2155
sbin/build.sh:# shellcheck disable=SC2155,SC2153,SC2038,SC1091,SC2116,SC2086

its usually some terrible eval

sbin/build.sh Outdated Show resolved Hide resolved
sbin/build.sh Outdated Show resolved Hide resolved
judovana and others added 2 commits June 13, 2024 12:19
Co-authored-by: Martijn Verburg <[email protected]>
Co-authored-by: Martijn Verburg <[email protected]>
@judovana
Copy link
Contributor Author

@karianna thanx. The smallest I can do as a gratitude for your patience, is to sign myself up for some english class over summer:( do x does.. what a shame...

@sxa sxa requested a review from andrew-m-leonard June 17, 2024 13:40
Copy link
Member

@sxa sxa left a comment

Choose a reason for hiding this comment

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

I'd like @andrew-m-leonard to review this one too since it's part of the setup for CycloneDX SBOM generation, so adding him as an approver. LGTM though. (Reviewing this as a "Comment" instead of "Approve" so that it doesn't get a second approval and merged until Andrew has approved!
Coincidentally this is related to a shellcheck exclusion that I queried in another PR today.

@judovana
Copy link
Contributor Author

It is clear bug. If it do not fail asap, it will die later, just in not debugg-able exetption.

Thanx for eyball!

Copy link
Contributor

@andrew-m-leonard andrew-m-leonard left a comment

Choose a reason for hiding this comment

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

I think it's good, I didn't realise it did that...!

@andrew-m-leonard andrew-m-leonard merged commit be1dc31 into adoptium:master Jun 20, 2024
28 checks passed
@judovana
Copy link
Contributor Author

ty!

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.

4 participants