This repository has been archived by the owner on Oct 22, 2020. It is now read-only.
Move ddrgen step after building VM #175
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mikezhang1234567890
force-pushed
the
ddr-compile
branch
from
a3ab77e to
e6e70c6
Compare
keithc-ca
suggested changes
Aug 13, 2018
closed/OpenJ9.gmk
Outdated
| @$(ECHO) Copying j9ddr.dat | ||
| @$(ECHO) Running ddrgen to create j9ddr.dat, then copying it to lib directory | ||
| $(MAKE) -C $(OUTPUT_ROOT)/vm/ddr -f run_omrddrgen.mk VERSION_MAJOR=9 OPENJDK_VERSION_NUMBER_FOUR_POSITIONS=$(VERSION_NUMBER_FOUR_POSITIONS) \ | ||
| CC="$(CC)" CXX="$(CXX)" $(EXPORT_MSVS_ENV_VARS) |
There was a problem hiding this comment.
This work should be captured in a rule (as in ibmruntimes/openj9-openjdk-jdk8#110) so this work only happens as necessary.
keithc-ca
reviewed
Aug 14, 2018
closed/OpenJ9.gmk
Outdated
| @@ -109,6 +109,21 @@ OPENJ9_MISC_FILES := \ | |||
| OPENJ9_NOTICE_FILES := openj9-notices.html | |||
| OPENJ9_REDIRECTOR := redirector/$(LIBRARY_PREFIX)jvm_b156$(SHARED_LIBRARY_SUFFIX) | |||
|
|
|||
| OPENJ9_DDR_FILES := \ | |||
| ifeq (true,$(OPENJ9_ENABLE_DDR)) | |||
There was a problem hiding this comment.
I won't be surprised if this doesn't behave well when DDR is not enabled. Have you tried that scenario?
mikezhang1234567890
changed the title
mikezhang1234567890
force-pushed
the
ddr-compile
branch
from
adf38e2 to
9648e29
Compare
keithc-ca
reviewed
Aug 14, 2018
closed/OpenJ9.gmk
Outdated
| OPENJDK_VERSION_NUMBER_FOUR_POSITIONS=$(VERSION_NUMBER_FOUR_POSITIONS) \ | ||
| CC="$(CC)" CXX="$(CXX)" \ | ||
| $(EXPORT_MSVS_ENV_VARS) | ||
| OPENJ9_DDR_FILES += j9ddr.dat |
There was a problem hiding this comment.
This line cannot start with a tab else it will be considered to be part of the script to produce run-ddrgen .
Also, there should be a rule
.PHONY : run-ddrgen
9648e29 to
48042bf
Compare
mikezhang1234567890
changed the title
mikezhang1234567890
force-pushed
the
ddr-compile
branch
5 times, most recently
from
cbe7634 to
16f27bc
Compare
keithc-ca
reviewed
Aug 16, 2018
closed/OpenJ9.gmk
Outdated
| @@ -109,6 +109,22 @@ OPENJ9_MISC_FILES := \ | |||
| OPENJ9_NOTICE_FILES := openj9-notices.html | |||
| OPENJ9_REDIRECTOR := redirector/$(LIBRARY_PREFIX)jvm_b156$(SHARED_LIBRARY_SUFFIX) | |||
|
|
|||
| OPENJ9_DDR_FILES := | |||
There was a problem hiding this comment.
Please remove the trailing whitespace here and squash to a single commit.
Add j9ddr.dat as a target run after building the VM so that ddrgen is moved outside of the build VM step. This helps resolve DDR dependencies and ddrgen is only run if the file does not exist or needs to updating. Signed-off-by: mikezhang <[email protected]>
mikezhang1234567890
force-pushed
the
ddr-compile
branch
from
16f27bc to
28cc270
Compare
keithc-ca
approved these changes
Aug 16, 2018
|
Jenkins test sanity win,zlinux |
2 similar comments
|
Jenkins test sanity win,zlinux |
|
Jenkins test sanity win,zlinux |
|
The third time's the charm? |
|
The zLinux failure is eclipse-openj9/openj9#2364; ddrgen completed before the Windows failure, so neither is due to this change: merging. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move the run ddrgen step outside of the building VM step in order to
resolve DDR dependencies better.
This PR moves the run ddrgen step to after the VM is built, and needs to be delivered together with eclipse-openj9/openj9#2588.
Signed-off-by: mikezhang [email protected]