RFR: JDK-8189955 Configuration validation is broken for some types of paths
Erik Joelsson
erik.joelsson at oracle.com
Thu Oct 26 09:09:28 UTC 2017
Hello,
On 2017-10-26 10:56, Magnus Ihse Bursie wrote:
>
> On 2017-10-26 10:09, Erik Joelsson wrote:
>> I see why this huge check becomes necessary for a valid comparison,
>> but it's quite a bit of extra complexity added as well. I was worried
>> about the extra overhead on an already slow platform so I took the
>> time to measure. While it's certainly measurable it's hardly
>> noticeable. On a fast machine, the difference is around 50-100ms on
>> repeated make invocations (baseline 1.85s). On my slow laptop it's
>> 300-400ms, but the baseline is already over 7 seconds so as said,
>> hardly noticeable.
> The alternative is to drop the check entirely. I did some source
> control archeology now, to figure out the origin of this check. It has
> grown a bit over the years to handle more complex cases, but the
> original test was introduced by me in JDK-8076060, which was the big
> rewrite that created Init.gmk and the current "bootstrapping" process
> of the build.
>
> I can't, at least now, figure out what was the driving need behind
> that check. It might been that I had collected some complaints about
> odd broken setups that "the build system should check for that!!! i've
> wasted hours!!!" but never got a separate bug. Or I might have
> encountered issues myself while developing the patch. Or I just
> thought it was prudent to check.
>
> In any case, I think it's fully viable by now to remove the check in
> its entirety. Do you agree?
>
Looking back there doesn't seem to be a corresponding check previous to
JDK-8076060. The original comment "Sanity check the spec file, so it
matches this source code" indicates a fear of someone trying to use a
spec file from a different source tree. That would certainly have weird
effects, but I don't think the build should be held responsible. The
previous check was pretty cheap so could be argued worth it. This check
has on the other hand caused quite a bit of problems for us and given
the increased complexity necessary for keeping it in there, I would
agree with your conclusion. It's better to just drop it.
>> That said, line 392 is easily expressible using the make filter
>> function instead of going to the shell. All the other shell calls
>> seem to be necessary however.
>>
>> Just thinking some more. Would it be possible to do something like
>> "cd DIR && pwd" for both dirs and just compare the output of that
>> instead? If that works it would seem like way fewer shell calls and
>> simpler logic.
> Unfortunately, that will not present a canonical representation of the
> directory. In cygwin, both /cygdrive/c/cygwin/home/user and /home/user
> is the same directory, but cygwin will not rewrite the path of any of
> them using that formula. :-( So for better or for worse, I think the
> method that has hardened over the years in our configure script is a
> battle-proven way to create a canonical path representation on all
> platforms.
>
Ah, of course, that's the tricky situation to overcome.
/Erik
More information about the build-dev
mailing list