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
