Re: [PATCH v2 3/5] doc: remove unnecessary rm instances
From: Felipe Contreras <hidden>
Date: 2021-05-17 10:42:37
Jeff King wrote:
On Sat, May 15, 2021 at 07:04:05AM -0500, Felipe Contreras wrote:quoted
Jeff King wrote:
quoted
quoted
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.
Ahh, I see. It's quite a bit more work, but sure, I can do that. -- Felipe Contreras