Sun 22 May 2016 07:24:42 PM UTC, comment #11:
The patch is attached to the ticket separate from my
comment:
https://savannah.gnu.org/bugs/download.php?file_id=37212
For the first change, it makes no sense to me to propagate a
timestamp on a remade rule to an unremade rule. Which the
f_mtime() function is doing (verified in debugger) when
called from remake.c:179 (of git tree 4c9e10f). We could
try to ensure that f_time is never called when it is not
safe to propagate a timestamp to all the double colon rules,
but it is more robust to make f_mtime() itself do the check
so that f_mtime() can be used more freely.
For the second change, notice that the problematic timestamp
propagation was happening at remake.c:179 but, in the
non-parallel case, a loop forcing all the double colon
targets to be remade before that happens at remake.c:358.
But for the parallel case, that loop only queues jobs and
all of the rules will be considered later after the next
reap_children() after the timestamp mistake occurs.
The loop is referenced in the changelog:
I don't know what bug 1476 was and don't know how to find it
on the internet, but I suspect it was to fix an issue
regarding an insufficient number of double-colon rules being
run due to the timestamp propagation in f_mtime. I could be
wrong, but I'll believe this loop is unnecessary until
somebody explains to me why we need it. I left it in only
to make my change more conservative in that it only impacts
parallel builds with double colons.
Disclosure: I do not understand why other, earlier, calls to
f_mtime() have not been an issue.
Now, supposing I'm wrong about something or that I
misunderstand something, it does not matter much, because it
is easy to prove that my change does not affect any of
make's behavior unless it is running parallel and it
includes double-colons. A situation that is currently badly
broken and a situation, which, empirically, my change fixes.
So I feel justified in submitting the patch even with an
incomplete understanding.
|
Sun 22 May 2016 04:44:05 AM UTC, comment #10:
"Here is a one-line fix: double-colons-bail-bug44742.patch"
1) There is no file attached.
2) Could you please explain the thinking behind those 2 patches.
Thanks
|
Sat 21 May 2016 09:36:13 PM UTC, comment #9:
Thanks for those patches, Joe. Fixed for the next release of GNU make.
|
Fri 20 May 2016 10:07:34 AM UTC, comment #8:
Turns out that the original issue for this bug was separate than the one I addressed in my first patch.
Here is a one-line fix: double-colons-bail-bug44742.patch
Both patches need to be applied for double colon rules to work properly in parallel builds.
|
Fri 20 May 2016 06:23:12 AM UTC, comment #7:
This patch (double-colons-parallel-bug44742.patch) corrects the attached test case (double_colon_parallel_test.diff).
I believe it fixes this bug. Test and let me know.
(file #37211)
|
Sun 09 Aug 2015 02:40:30 PM UTC, comment #6:
It's a bug. It should definitely be possible to use double-colon rules with parallel builds.
|
Sun 09 Aug 2015 02:35:28 PM UTC, comment #5:
It definitely does ruin the build. My question is, is this by design or simply a long-standing bug in GNU make? It doesn't happen on BSD make, for instance.
|
Wed 05 Aug 2015 09:46:34 AM UTC, comment #4:
Double-colon may not "play" nicely with parallel-execution.
The following example shows one situation, where parallel execution may skip over a dependency (i.e. NOT building it at all), because it was linked in via a double-colon.
As shown later, this occurs only for parallel execution, so you should choose EITHER one of them, parallel-execution OR double-colon.
Using BOTH (parallel-execution AND double-colon), may ruin your build.
Running
We get:
Where, running
We get:
Or, to put simply, running
We get:
Where, running
We get:
|
Mon 03 Aug 2015 01:46:14 AM UTC, comment #3:
Thanks for taking a look! I tried patching my local GNU make 4.1 per your suggestion, and it hasn't helped. Here is a fuller set of steps to repro:
<code>
# initial setup
cpanm --dev --look PDL
cpanm ExtUtils::MakeMaker@7.05_19 # 7.05* has double-level
perl Makefile.PL
cd Basic/Core
# first time, and show failure with -j4 linkext
make -j4 linkext
ls ../../blib/arch/auto/PDL/Core/Core.so # not found
# keep old files to save time
cp Version.pm Version.pm.KEPT ; cp Types.pm Types.pm.KEPT
# show works with -j4 dynamic
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j4 dynamic ; ls ../../blib/arch/auto/PDL/Core/Core.so
# show works with -j1 linkext
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j1 linkext; ls ../../blib/arch/auto/PDL/Core/Core.so
# show fails again/still with -j4 linkext
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j4 linkext; ls ../../blib/arch/auto/PDL/Core/Core.so
</code>
This is a problem for EUMM since it's a bug that affects already-existing GNU make, so I'm going to have to find a way to make this not happen for current GNU make. However, it also seems like it would be worth finding and fixing this.
(file #34568)
|
Sun 02 Aug 2015 06:29:49 PM UTC, comment #2:
Thanks for taking a look! I tried patching my local GNU make 4.1 per your suggestion, and it hasn't helped. Here is a fuller set of steps to repro:
<code>
# initial setup
cpanm --dev --look PDL
cpanm ExtUtils::MakeMaker@7.05_19 # 7.05* has double-level
perl Makefile.PL
cd Basic/Core
# first time, and show failure with -j4 linkext
make -j4 linkext
ls ../../blib/arch/auto/PDL/Core/Core.so # not found
# keep old files to save time
cp Version.pm Version.pm.KEPT ; cp Types.pm Types.pm.KEPT
# show works with -j4 dynamic
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j4 dynamic ; ls ../../blib/arch/auto/PDL/Core/Core.so
# show works with -j1 linkext
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j1 linkext; ls ../../blib/arch/auto/PDL/Core/Core.so
# show fails again/still with -j4 linkext
rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; make -j4 linkext; ls ../../blib/arch/auto/PDL/Core/Core.so
</code>
This is a problem for EUMM since it's a bug that affects already-existing GNU make, so I'm going to have to find a way to make this not happen for current GNU make. However, it also seems like it would be worth finding and fixing this.
|
Sat 11 Jul 2015 06:02:45 PM UTC, comment #1:
Sorry but I can't understand your repro case, or at least I can't reproduce it. The makefile you link here doesn't appear to be a stand-alone repro case; if I run it by itself I just get errors about missing Makefile.PL.
If I install cpanm then run the command in your "full repro by" line, it builds (after I install some other Perl prerequisites) but I don't get the error you see: running with/without -j4 it always successfully creates the Core.so file.
If I try the commands you list below, removing, running make clean, etc. (it's not clear whether the "Makefile" you're copying is the one you attached here or the one left over from "make clean", but neither seem to work for me) I don't have any .KEPT files and I get errors about not being able to locate PDL/Types.pm.
However, someone sent me a fix which may be related to this. If you edit the file remake.c in the GNU make sources around line 351 and change:
to this:
(Note there is another similar if-statement earlier in the function which already has "status = new"; they should both be that way.)
See if that change helps; I cannot reproduce your problem so I cannot check it myself.
|
Mon 06 Apr 2015 12:08:02 AM UTC, original submission:
If you do "make -j4 linkext", it will not link the *.so. If you do "make -j4 dynamic", or "make -j1 linkext", it will.
This is the whole of the "linkext" rule:
linkext :: dynamic
$(NOECHO) $(NOOP)
I attach the fuller log, the (cut-down) Makefile, the "remake -x" output of each case.
Full repro by: cpanm --dev --look PDL; cd Basic/Core; make linkext
Log:
$ make -v
GNU Make 4.1
[...]
$ remake -v
GNU Make 4.1+dbg0.91
[...]
$ rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; remake -x linkext >output.linkext ; l ../../blib/arch/auto/PDL/Core/Core.so
Using new 64bit index support
Using new 64bit index support
ls: cannot access ../../blib/arch/auto/PDL/Core/Core.so: No such file or directory
$ rm -rf ../../blib ; make clean >/dev/null; mv Makefile.old Makefile ; cp Version.pm.KEPT Version.pm ; cp Types.pm.KEPT Types.pm ; remake -x dynamic >output.dynamic ; l ../../blib/arch/auto/PDL/Core/Core.so
Using new 64bit index support
Using new 64bit index support
-rwxr-xr-x. 1 user user 168335 Apr 6 00:42 ../../blib/arch/auto/PDL/Core/Core.so
|