| Message ID | 559CE58A.90706@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 8 July 2015 at 14:25, Carlos O'Donell <carlos@redhat.com> wrote: > This is a super annoying failure mode that happens when `make` fails or > is not run. The fix is as follows, and I'll check it in shortly. > > 2015-07-08 Carlos O'Donell <carlos@redhat.com> > > * Makefile ($(objpfx)check-local-headers.out): > Redirect input from /dev/null. > > diff --git a/Makefile b/Makefile > index 658ccfa..c88b2e5 100644 > --- a/Makefile > +++ b/Makefile > @@ -262,7 +262,7 @@ endif > > $(objpfx)check-local-headers.out: scripts/check-local-headers.sh > AWK='$(AWK)' scripts/check-local-headers.sh \ > - "$(includedir)" "$(objpfx)" > $@; \ > + "$(includedir)" "$(objpfx)" < /dev/null > $@; \ > $(evaluate-test) That looks like a hack. Does it hang because $(objpfx) and $(includedir) are not set? A better fix ought to be to ensure that either 'make check' invokes make (and hence sets things up for the check target) or it fails early and cleanly, i.e. irrespective of whether the check-local-headers test is run or not. Siddhesh
"Carlos O'Donell" <carlos@redhat.com> writes: > diff --git a/Makefile b/Makefile > index 658ccfa..c88b2e5 100644 > --- a/Makefile > +++ b/Makefile > @@ -262,7 +262,7 @@ endif > > $(objpfx)check-local-headers.out: scripts/check-local-headers.sh > AWK='$(AWK)' scripts/check-local-headers.sh \ > - "$(includedir)" "$(objpfx)" > $@; \ > + "$(includedir)" "$(objpfx)" < /dev/null > $@; \ > $(evaluate-test) This isn't the only problem with check-local-headers. It should really make sure all *.dt files have been converted to *.d files first. Andreas.
Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> writes: > That looks like a hack. Does it hang because $(objpfx) and > $(includedir) are not set? No, it hangs because */*.{o,os,oS}.d expands to nothing. Andreas.
On 07/08/2015 05:44 AM, Siddhesh Poyarekar wrote: > On 8 July 2015 at 14:25, Carlos O'Donell <carlos@redhat.com> wrote: >> This is a super annoying failure mode that happens when `make` fails or >> is not run. The fix is as follows, and I'll check it in shortly. >> >> 2015-07-08 Carlos O'Donell <carlos@redhat.com> >> >> * Makefile ($(objpfx)check-local-headers.out): >> Redirect input from /dev/null. >> >> diff --git a/Makefile b/Makefile >> index 658ccfa..c88b2e5 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -262,7 +262,7 @@ endif >> >> $(objpfx)check-local-headers.out: scripts/check-local-headers.sh >> AWK='$(AWK)' scripts/check-local-headers.sh \ >> - "$(includedir)" "$(objpfx)" > $@; \ >> + "$(includedir)" "$(objpfx)" < /dev/null > $@; \ >> $(evaluate-test) > > That looks like a hack. Does it hang because $(objpfx) and > $(includedir) are not set? A better fix ought to be to ensure that > either 'make check' invokes make (and hence sets things up for the > check target) or it fails early and cleanly, i.e. irrespective of > whether the check-local-headers test is run or not. It's not a hack. The script reads from stdin for the list of *.d files that were created by the build, but if there are no *.d files the shell expansion is empty and that forces the script to read from the parent's inherited stdin. Since the parent never writes anything to stdin the awk script blocks on the read forever. As Andreas notes it should dep on the *.d files existing, and that is true. However, even if that were fixed we should still read from /dev/null and fail the test even if all the *.d files were deleted between the time they were created and the script ran (robust). Cheers, Carlos.
On 8 July 2015 at 18:16, Carlos O'Donell <carlos@redhat.com> wrote: > It's not a hack. The script reads from stdin for the list of *.d > files that were created by the build, but if there are no *.d files > the shell expansion is empty and that forces the script to read from > the parent's inherited stdin. Since the parent never writes anything > to stdin the awk script blocks on the read forever. As Andreas notes > it should dep on the *.d files existing, and that is true. However, > even if that were fixed we should still read from /dev/null and > fail the test even if all the *.d files were deleted between the > time they were created and the script ran (robust). Right, I misread what that target was doing. In any case, the right fix here would be to make the check-local-headers target depend on *.d (one of them I guess?) in a way that they're always generated before the script is run. Siddhesh
On 07/08/2015 09:09 AM, Siddhesh Poyarekar wrote: > On 8 July 2015 at 18:16, Carlos O'Donell <carlos@redhat.com> wrote: >> It's not a hack. The script reads from stdin for the list of *.d >> files that were created by the build, but if there are no *.d files >> the shell expansion is empty and that forces the script to read from >> the parent's inherited stdin. Since the parent never writes anything >> to stdin the awk script blocks on the read forever. As Andreas notes >> it should dep on the *.d files existing, and that is true. However, >> even if that were fixed we should still read from /dev/null and >> fail the test even if all the *.d files were deleted between the >> time they were created and the script ran (robust). > > Right, I misread what that target was doing. In any case, the right > fix here would be to make the check-local-headers target depend on *.d > (one of them I guess?) in a way that they're always generated before > the script is run. I agree that `make check` should effectively run `make` if the dependencies are not yet met, but we aren't there yet, and I see a whole host of other failures to fix. It's also not obvious to me how I would depend on *all* *.d files in that makefile target, it doesn't seem trivial to compute them. The fix to read from /dev/null seems like the lowest cost immediate solution that yields a `make check` failure rather than a hang (which is terrible for automated test systems also). If nobody objects I'll check it in now to p Cheers, Carlos.
Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> writes: > Right, I misread what that target was doing. In any case, the right > fix here would be to make the check-local-headers target depend on *.d > (one of them I guess?) in a way that they're always generated before > the script is run. It would need to depend on a target that recurses through all subdirs. The %.d:%.dt rule is triggered by include $(+depfiles). Andreas.
"Carlos O'Donell" <carlos@redhat.com> writes: > The fix to read from /dev/null seems like the lowest cost immediate > solution that yields a `make check` failure rather than a hang (which > is terrible for automated test systems also). Alternatively, add /dev/null to the awk command line. That would also make the comment true: # OK if *.os is missing. Andreas.
On Wed, 8 Jul 2015, Andreas Schwab wrote: > Siddhesh Poyarekar <siddhesh.poyarekar@gmail.com> writes: > > > Right, I misread what that target was doing. In any case, the right > > fix here would be to make the check-local-headers target depend on *.d > > (one of them I guess?) in a way that they're always generated before > > the script is run. > > It would need to depend on a target that recurses through all subdirs. > The %.d:%.dt rule is triggered by include $(+depfiles). I'd say that check-local-headers, instead of being a top-level test at all, should be a test present automatically in each subdirectory that checks only files in that subdirectory (and so doesn't need to depend on anything that recurses, only on something within that directory). See what I said in <https://sourceware.org/ml/libc-alpha/2014-01/msg00197.html> about eliminating top-level tests.
diff --git a/Makefile b/Makefile index 658ccfa..c88b2e5 100644 --- a/Makefile +++ b/Makefile @@ -262,7 +262,7 @@ endif $(objpfx)check-local-headers.out: scripts/check-local-headers.sh AWK='$(AWK)' scripts/check-local-headers.sh \ - "$(includedir)" "$(objpfx)" > $@; \ + "$(includedir)" "$(objpfx)" < /dev/null > $@; \ $(evaluate-test) ifneq ($(PERL),no)