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

Add support for windows platforms #19

Merged
merged 1 commit into from
Dec 18, 2017
Merged

Conversation

vsebe
Copy link
Contributor

@vsebe vsebe commented Dec 14, 2017

Signed-off-by: Violeta Sebe vsebe@ca.ibm.com

@vsebe vsebe requested a review from pshipton December 14, 2017 18:34
@vsebe
Copy link
Contributor Author

vsebe commented Dec 14, 2017

Depends on eclipse-openj9/openj9#829

-server KNOWN
-client IGNORE
-j9vm KNOWN
-hotspot IGNORE
Copy link
Member

@pshipton pshipton Dec 14, 2017

Choose a reason for hiding this comment

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

I suppose this just copies what is done for other platforms, but comparing with the previous values, it seems the ignores should be:

-server IGNORE
-client IGNORE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should [re]add them to all jvm.cfg files?

Copy link
Member

Choose a reason for hiding this comment

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

I think that can be done separately from this commit.

OPENJ9_PLATFORM_CODE=wa64
OPENJ9_BUILDSPEC="win_x86-64_cmprssptrs"
else
AC_MSG_ERROR([Unsupported OpenJ9 platform ${OPENJDK_BUILD_OS}, contact support team!])
Copy link
Member

Choose a reason for hiding this comment

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

Please remove ", contact support team!". The "support team" for OpenJ9 are the git repos, where an issue would be opened if appropriate.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

OPENJ9_PLATFORM_CODE=wa64
OPENJ9_BUILDSPEC="win_x86-64_cmprssptrs"
else
as_fn_error $? "Unsupported OpenJ9 platform ${OPENJDK_BUILD_OS}, contact support team!" "$LINENO" 5
Copy link
Member

Choose a reason for hiding this comment

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

Please remove ", contact support team!". The "support team" for OpenJ9 are the git repos, where an issue would be opened if appropriate.

@@ -36813,7 +36813,11 @@ $as_echo "$OUTPUT_DIR_IS_LOCAL" >&6; }
# At the end, call the custom hook. (Dummy macro if no custom sources available)

# Add the J9VM vm lib directory into native LDFLAGS_JDKLIB path
LDFLAGS_JDKLIB="${LDFLAGS_JDKLIB} -L${JDK_OUTPUTDIR}/../vm"
if test "x$OPENJDK_BUILD_OS_ENV" = xwindows.cygwin; then
LDFLAGS_JDKLIB="${LDFLAGS_JDKLIB} -libpath:${JDK_OUTPUTDIR}/../vm/lib"
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why the directory is different from other platforms.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is on win

Copy link
Member

Choose a reason for hiding this comment

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

I realize its on Windows, but what is it about Windows that makes the directory different?

Copy link
Member

Choose a reason for hiding this comment

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

Seems on Windows you link to the .lib files which are in the lib subdirectory, while on other platforms you link to the .so files which are in the root vm directory.

@@ -258,7 +271,10 @@ run-preprocessors-j9 : stage-j9 \

build-j9 : run-preprocessors-j9
@$(ECHO) Compiling OpenJ9 in $(OUTPUT_ROOT)/vm
(export OMR_DIR=$(OUTPUT_ROOT)/vm/omr OPENJ9_BUILD=true && cd $(OUTPUT_ROOT)/vm && $(MAKE) $(MAKEFLAGS) JAVA_VERSION=80 VERSION_MAJOR=8 all)
(export OPENJ9_BUILD=true $(EXPORT_NO_USE_MINGW) $(EXPORT_MSVS_ENV_VARS) \
Copy link
Member

Choose a reason for hiding this comment

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

Why is OMR_DIR removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not required.

@@ -30,7 +30,7 @@ clean-j9vm:
j9vm-build :
+($(CD) $(SRC_ROOT)/closed && $(MAKE) -f OpenJ9.gmk SPEC=$(SPEC) build-j9)

j9vm-compose-buildjvm : j9vm-build $(J9JCL_GENSRC_MAKEFILE)
j9vm-compose-buildjvm : j9vm-build
Copy link
Member

Choose a reason for hiding this comment

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

Why is $(J9JCL_GENSRC_MAKEFILE) removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not exist.

@vsebe vsebe force-pushed the openj9 branch 2 times, most recently from 2597cd5 to 5f53bc3 Compare December 14, 2017 21:14
@vsebe
Copy link
Contributor Author

vsebe commented Dec 14, 2017

I fixed jvm.cfg, custom-hook.m4 and generated-configure.sh; ready for review, thx

@@ -34,5 +34,10 @@
# "-XXaltjvm=<jvm_dir>" option, but that too is unsupported
# and may not be available in a future release.
#
-server KNOWN
-j9vm KNOWN
-hotspot IGNORE
Copy link
Member

Choose a reason for hiding this comment

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

You can remove the IGNORE for hotspot, classic, native, and green, since they didn't exist before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@vsebe
Copy link
Contributor Author

vsebe commented Dec 15, 2017

@pshipton cleaned up jvm.cfg as suggested, thx.

.gitignore Outdated
@@ -0,0 +1,5 @@
/.project
build/
Copy link
Member

Choose a reason for hiding this comment

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

To be consistent with the other directories and openj9-openjdk-jdk9, build/ should start with a slash

Signed-off-by: Violeta Sebe <vsebe@ca.ibm.com>
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.

None yet

2 participants