On Thu, 3 Dec 2020 22:39:00 GMT, Magnus Ihse Bursie <[email protected]> wrote:
>> 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.
Fair enough.
>> 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?
>
> I agree it is a bit hairy. Perhaps not ideal; even though this is a rewrite
> it has suffered some legacy "lava flow" itself, being in development for so
> long.
>
> The idea here is. that NOFIXPATH (or not) as $4 is passed to
> UTIL_FIXUP_EXECUTABLE. If it is set, then UTIL_FIXUP_EXECUTABLE will never
> prepend FIXPATH. Otherwise, UTIL_FIXUP_EXECUTABLE will do the check you are
> requesting, and see if we are on Windows and it is a non-unix-aware
> executable, and if so, prepend FIXPATH.
>
> So when we come back to UTIL_LOOKUP_PROGS, if we don't have NOFIXPATH, and we
> are on Windows (FIXPATH is non-empty) and the path we got from
> UTIL_FIXUP_EXECUTABLE starts with FIXPATH, then we remove all instances of
> FIXPATH, and prepend a single FIXPATH instance.
>
> While seems like unneccessary work. I ran into situations where I got a
> double FIXPATH prefix. It might have been the result of some other bug that
> is fixed by now, but costs very little to keep this as a safeguard. It should
> get a comment, though, describing what it does.
Comment is good for me, thanks.
-------------
PR: https://git.openjdk.java.net/jdk/pull/1597
