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

Use $(PHASE_MAKEDIRS) for the Java phase for consistency #270

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

keithc-ca
Copy link
Member

The pre-hook and post-hook in CompileJavaModules.gmk were removed by

  • 8258407 Split up CompileJavaModules.gmk into make/modules/$M/Java.gmk

and the include directive for Java.gmk uses an absolute path leaving no path to customizations. This updates Main.gmk to define an include path based on PHASE_MAKEDIRS for make as was done for other phases in

  • 8252998 ModuleWrapper.gmk doesn't consult include path

This is required by part of the anticipated fix for eclipse-openj9/openj9#11464.

The pre-hook and post-hook in CompileJavaModules.gmk were removed by

* 8258407 Split up CompileJavaModules.gmk into make/modules/$M/Java.gmk

and the include directive for Java.gmk uses an absolute path leaving no
path to customizations. This updates Main.gmk to define an include path
based on PHASE_MAKEDIRS for make as was done for other phases in

* 8252998 ModuleWrapper.gmk doesn't consult include path

Signed-off-by: Keith W. Campbell <[email protected]>
@keithc-ca
Copy link
Member Author

Note that I've asked @adamfarley to propose this change upstream.

@pshipton
Copy link
Member

jenkins compile win,osx,zlinux jdknext

@adamfarley
Copy link

Upstream material all set:

Bug: https://bugs.openjdk.java.net/browse/JDK-8259942
PR: openjdk/jdk#2134
Mailing list: https://mail.openjdk.java.net/pipermail/build-dev/2021-January/030042.html

@pshipton pshipton merged commit ed744b1 into ibmruntimes:openj9 Jan 18, 2021
@keithc-ca keithc-ca deleted the java_phase branch January 18, 2021 19:39
@adamfarley
Copy link

@keithc-ca - FYI: this change was just merged upstream.

@keithc-ca
Copy link
Member Author

Thanks, @adamfarley; I noticed it while reviewing #290.

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