On Thu, 3 Dec 2020 15:20:07 GMT, Erik Joelsson <er...@openjdk.org> wrote:

>> Magnus Ihse Bursie has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Missed my last bugfix for fixpath.sh...
>
> 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?

If we are creating a buildjdk (which really means we are cross-compiling), we 
must not meddle with the Windows runtime dll:s, since they are for the target 
CPU architecture. To be *fully* correct, we should really turn the Windows 
runtime dll:s into a "normal"/BUILD_ split version, like with SYSROOT etc. 

But since this is just used locally for building, I'm assuming the build system 
has these runtime DLLs installed already. There seems to be no point in wasting 
time preparing a buildjdk with files that are just bundled for installation.

Maybe there's a better way to express this, but the only thing I could get a 
hold of for the fact that we are creating a build-JDK (that is, not targeting 
the normal OPENJDK_TARGET_CPU) was the CREATING_BUILDJDK variable.

> 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?

I assume you are right. Even though it bothers me to my bone that we just check 
out the complete source for testing, instead of building a proper test image. 
:) I'll fix.

> 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?

Yes, I tried that first. And pulled my hair a couple of times. Turns out there 
is no way (that I could find, at least) to get WriteFile to write multiple 
lines.

> 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?

Yes. And this applies to the other fix above as well. Hm. Maybe it is enough to 
avoid copying them here, and then I don't need the patch in CreateJmods.gmk

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

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

Reply via email to