On Thu, 3 Dec 2020 15:30:15 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:

>> For the build to work on Windows, we need a unix compatibility layer (known 
>> as the "winenv" in the build system). This can be e.g. Cygwin or Msys. The 
>> build system then needs to adapt various aspect to get the build to work in 
>> this winenv. Over time, our current solutions (which were never that 
>> well-designed) has collapsed into an unmaintainable mess.
>> 
>> This rewrite takes on a fresh approach, by giving up on the native 
>> "fixpath.exe" converter, and instead relying on a platform-independent shell 
>> script "fixpath.sh", which can dynamically adapt to the current winenv. It 
>> also provides a proper framework on how to categorize, and support, 
>> different winenvs. As a result, we now easily support Cygwin, Msys2, WSL1 
>> and WSL2 (the latter is unfortunately not mature enough to be able to 
>> compile the JDK).
>> 
>> Furthermore, this rewrite removes all the kludges and hacks that were put in 
>> place all over the code base, and consolidates all tricky part of handling 
>> the winenv to basically two places: setting up in configure, and run-time 
>> handling by fixpath.sh.
>> 
>> This patch also cleans up our handling of how we detect tools in configure, 
>> and makes a proper framework for cross-compilation on Windows.
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Missed my last bugfix for fixpath.sh...

Overall this is very nice. Some comments and questions inline.

make/CreateJmods.gmk line 172:

> 170:   endif
> 171: else # not java.base
> 172:   ifeq ($(call isTargetOs, windows)+$(CREATING_BUILDJDK), true+false)

This looks strange. Why would CREATING_BUILDJDK be relevant here?

make/TestImage.gmk line 34:

> 32: 
> 33: ifeq ($(call isTargetOs, windows), true)
> 34:   FIXPATH_COPY := $(TEST_IMAGE_DIR)/bin/fixpath.sh

If fixpath.sh is now a static shell script in make/scripts, there is no need to 
transport it through the test image, is there?

make/autoconf/toolchain.m4 line 838:

> 836:     else
> 837:       if test "x$TOOLCHAIN_TYPE" = xmicrosoft; then
> 838:         # If we got no  devkit, we need to go hunting for the proper env

Double space

make/autoconf/util_paths.m4 line 401:

> 399:             if test "x$4" != xNOFIXPATH; then
> 400:               [ if [[ $FIXPATH != "" && $result =~ ^"$FIXPATH " ]]; then 
> ]
> 401:                 result="\$FIXPATH ${result#"$FIXPATH "}"

Maybe I'm missing something, but is this unconditionally adding fixpath to any 
executable based just on if FIXPATH is set? Shouldn't there be a conditional on 
if the executable is Windows or Unix type?

make/autoconf/util_paths.m4 line 482:

> 480: 
> ###############################################################################
> 481: # Add FIXPATH prefix to variable. Normally this is done by 
> UTIL_LOOKUP_PROGS
> 482: # or UTIL_FIXUP_EXECUTABLE, but in some circumstances this have to be 
> done

this _has_ to be done

make/common/JavaCompilation.gmk line 232:

> 230:       $1_JAVAC_PORT_FILE := $$(call FixPath, 
> $$(JAVAC_SERVER_DIR)/server.port)
> 231: 
> 232:       # The servercmd specified how to launch the server. This will be 
> executed

specifies

make/common/JavaCompilation.gmk line 241:

> 239: 
> 240:       $$($1_JAVAC_SERVER_CONFIG): $$($1_CONFIG_VARDEPS_FILE)
> 241:  $(ECHO) portfile=$$($1_JAVAC_PORT_FILE) > $$@

Did you consider using WriteFile here?

make/modules/java.base/Copy.gmk line 48:

> 46: 
> ################################################################################
> 47: # Copy the microsoft runtime libraries on windows
> 48: ifeq ($(call isTargetOs, windows)+$(CREATING_BUILDJDK), true+false)

Is the problem here that we don't have build versions of the runtime libraries 
so we simply trust that they are present on the build host instead?

-------------

PR: https://git.openjdk.java.net/jdk/pull/1597

Reply via email to