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


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.


More information about the build-dev mailing list