Properly link omrsig with ddrgen

[babsingh]

If OMRPORT_OMRSIG_SUPPORT is defined globally, then ddrgen needs to link
to omrsig in order to compile properly.

Signed-off-by: Babneet Singh sbabneet@ca.ibm.com

[charliegracie]

Is there an equivalent CMake change required here as well? @dnakamura

[charliegracie]

@genie-omr build all

[charliegracie]

@babsingh have you had a change to look into CMake?

[babsingh]

@charliegracie i think we will need to update ddr/tools/ddrgen/CMakeLists.txt as follows:

add_executable(omr_ddrgen
	ddrgen.cpp
)

set_property(
	TARGET omr_ddrgen
	PROPERTY CXX_STANDARD 11
)

set(DDRGEN_LINK_LIBRARIES 
        omr_ddr_base
	omr_ddr_blobgen
	omr_ddr_ir
	omr_ddr_macros
	omr_ddr_scanner
	${OMR_PORT_LIB}
)

if(OMRPORT_OMRSIG_SUPPORT)
list(APPEND DDRGEN_LINK_LIBRARIES omrsig)
endif()

target_link_libraries(omr_ddrgen DDRGEN_LINK_LIBRARIES)

I will need to verify if the above the changes work. Is OMR build using CMake? If not, how to build OMR and OpenJ9 with CMake? I will push the CMake equivalent changes in a separate pull request.

[babsingh]

@charliegracie @dnakamura ^^^^

[charliegracie]

OMR is using CMake to build on most of the OMR CI builds.

[dnakamura]

IT should work because i believe the port lib links against omrsig, and in cmake, link dependencies are transitive

[babsingh]

In OpenJ9, port is linked to omrsig: runtime/port/CMakeLists.txt. In OMR, I don't see port being linked to omrsig: port/CMakeLists.txt. @dnakamura how is omrsig linked to port in OMR? is it linked transitively to port?

[charliegracie]

ping @dnakamura

[dnakamura]

Its possible that it isnt, but the CI build passes which would seem to indicate that it does somehow Realized that this pr doesnt enable the changes, so ddrgen will not need to link omrsig

[dnakamura]

I would patch the cmakelists to the following:

add_executable(omr_ddrgen
	ddrgen.cpp
)

set_property(
	TARGET omr_ddrgen
	PROPERTY CXX_STANDARD 11
)

target_link_libraries(omr_ddrgen
	omr_ddr_base
	omr_ddr_blobgen
	omr_ddr_ir
	omr_ddr_macros
	omr_ddr_scanner
	${OMR_PORT_LIB}
)
if(OMRPORT_OMRSIG_SUPPORT)
	target_link_libraries(omr_ddrgen omrsig)
endif()

[babsingh]

thanks @dnakamura. @charliegracie i have added the patch, suggested by Devin, into the pull request.

[babsingh]

I have created an issue to Verify OMR builds properly with OMRPORT_OMRSIG_SUPPORT enabled

[charliegracie]

@genie-omr build all

[babsingh]

@charliegracie zos failure seems to be an infra issue:

Error when executing always post condition:
hudson.remoting.ChannelClosedException: Channel "unknown": Remote call on ZISVJD03 failed. The channel is closing down or has closed down
	at hudson.remoting.Channel.call(Channel.java:948)
	at hudson.FilePath.act(FilePath.java:1036)
	at hudson.FilePath.act(FilePath.java:1025)
	at hudson.FilePath.deleteRecursive(FilePath.java:1230)
	at org.jenkinsci.plugins.workflow.steps.DeleteDirStep$Exec
        .....
        .....

[babsingh]

@charliegracie @rwy0717 should omrsig be treated as a shared or a static library?

[babsingh]

@charliegracie can these changes be merged?

[rwy7]

should omrsig be treated as a shared or a static library?
omrsig is a shared library.

[charliegracie]

The zOS build failed... I need to know why before I merge

[babsingh]

i have mentioned above that it is an infra issue. it should go away if you relaunch the zOS job.

[charliegracie]

@genie-omr build zos

[babsingh]

@charliegracie all checks have passed. this is ready to merge.