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.

Sign up for GitHub

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

Jump to bottom

Fail test-image-openj9 target when java -version fails #639

Merged
merged 1 commit into from
Jan 23, 2023
Merged

Fail test-image-openj9 target when java -version fails #639

merged 1 commit into from
Jan 23, 2023

Conversation

jdmpapin
Copy link
Contributor

Previously, the exit status would come from tee, which succeeded.

@jdmpapin jdmpapin mentioned this pull request Dec 14, 2022
Merged
@jdmpapin
Copy link
Contributor Author

Does this repo still need to require commit sign-off for some reason? I suspect that it doesn't, and that the failing check is a holdover

@keithc-ca
Copy link
Member

If you have good a reason that the check should be removed, then please capture it in an issue. Until we have agreement to remove the signed-off-by check, please update the commit message.

keithc-ca
keithc-ca reviewed Jan 5, 2023
View reviewed changes
closed/make/Main.gmk Outdated Show resolved Hide resolved
@jdmpapin
Copy link
Contributor Author

jdmpapin commented Jan 9, 2023

Until we have agreement to remove the signed-off-by check, please update the commit message.

Updated

If you have good a reason that the check should be removed, then please capture it in an issue.

Opened #644

@jdmpapin
Copy link
Contributor Author

jdmpapin commented Jan 9, 2023

Updated the copyright header

@jdmpapin
Copy link
Contributor Author

Changed cat to $(CAT) as requested.

@keithc-ca
Copy link
Member

Do you intend to offer equivalent changes for other jdk versions (11, 17, 19, 20, next)?

@jdmpapin
Copy link
Contributor Author

Oh, thanks, good catch! I should, yes. When I built various versions on my local machine under strace, I only noticed this java -version execution in Java 8, since there it was the only execution of a Java binary from within build/. I thought that it didn't happen in later versions, but it seems that it does and I just didn't notice it. I can see the rule is still there in Main.gmk, and I just re-ran Java 17 make under strace to see that java -version does run.

@jdmpapin
Copy link
Contributor Author

There a 'next' extensions repo? I don't see one.

@keithc-ca
Copy link
Member

There a 'next' extensions repo? I don't see one.

https://github.com/ibmruntimes/openj9-openjdk-jdk (without the numeric suffix).

@jdmpapin
Copy link
Contributor Author

ibmruntimes/openj9-openjdk-jdk11#614
ibmruntimes/openj9-openjdk-jdk17#177
ibmruntimes/openj9-openjdk-jdk19#62
ibmruntimes/openj9-openjdk-jdk20#10
ibmruntimes/openj9-openjdk-jdk#543

@keithc-ca keithc-ca mentioned this pull request Jan 19, 2023
Merged
@keithc-ca keithc-ca self-assigned this Jan 19, 2023
@jdmpapin
Copy link
Contributor Author

Updated this PR so that the changes are consistent with those for later JDK versions

keithc-ca
keithc-ca reviewed Jan 23, 2023
View reviewed changes
closed/make/Main.gmk Outdated Show resolved Hide resolved
@keithc-ca keithc-ca self-requested a review January 23, 2023 16:19
@jdmpapin
Fail test-image-openj9 target when java -version fails
0b414e4 
Previously, the exit status would come from tee, which succeeded.
@jdmpapin
Copy link
Contributor Author

Added quotes for consistency

@keithc-ca
Copy link
Member

Jenkins compile zlinux jdk8

@keithc-ca keithc-ca merged commit 21da581 into ibmruntimes:openj9 Jan 23, 2023
@jdmpapin jdmpapin deleted the version-exit-status branch April 21, 2023 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@keithc-ca keithc-ca keithc-ca approved these changes

Assignees

@keithc-ca keithc-ca

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jdmpapin @keithc-ca