On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:

Jeff King wrote:

quoted

On Fri, May 14, 2021 at 06:56:29AM -0500, Felipe Contreras wrote:

quoted

Commits 50cff52f1a (When generating manpages, delete outdated targets
first., 2007-08-02) and f9286765b2 (Documentation/Makefile: remove
cmd-list.made before redirecting to it., 2007-08-06) created these rm
instances for a very rare corner-case: building as root by mistake.

It's odd to have workarounds here, but nowhere else in the Makefile--
which already fails in this stuation, starting from
Documentation/technical/.

Aren't there tons more that you end up removing in the next patch? E.g.:

   doc.dep : $(docdep_prereqs) $(DOC_DEP_TXT) build-docdep.perl
  -       $(QUIET_GEN)$(RM) $@+ $@ && \
  -       $(PERL_PATH) ./build-docdep.perl >$@+ $(QUIET_STDERR) && \
  -       mv $@+ $@
  +       $(QUIET_GEN)$(PERL_PATH) ./build-docdep.perl >$@ $(QUIET_STDERR)

That does differ in that it removes $@+, too, but the premise is the
same (we know that $@+ could not be a problem, as we're about to
clobber it anyway).

I'm OK with getting rid of all of them, but it seems like it ought to be
happening all in this patch.

Yeah, but the rationale is different.

  1. $(RM) $@: these remove the target file because of permissions
     (i.e. root owned)
  2. $(RM) $@+ $@ && $(CODE) && mv $@+ $@: these are for interrupted builds

To get rid of #2 we need an alternative solution, like .DELETE_ON_ERROR,
to get rid of #1 we don't, we can just do it.

To get rid of the "mv", you need something like .DELETE_ON_ERROR. But
the "rm" in the second case has nothing to do with interrupted builds.
It is is doing the same nothing that the ones you are getting rid of
here are.

I.e., I was suggesting to get rid of the "rm" call in the hunk I showed
above, but leave the "mv" for the follow-on patch.

-Peff