diff mbox series

gas: rework timestamp preservation on doc/asconfig.texi

Message ID 20b1a608-1102-d462-eb99-c18e0b7d2f83@suse.com
State Accepted
Headers
Series gas: rework timestamp preservation on doc/asconfig.texi |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Jan Beulich July 31, 2023, 1:44 p.m. UTC 
  PR 28909

Sadly "cp -p", doing more than just preserving the time stamp, can fail
e.g. upon trying to preserve ownership (which we don't care about), as
can be observed on e.g. Cygwin. Replace the use of -p by a use of touch,
this way also only preserving modification time.
---
Interestingly that earlier commit (8034b0baeac1) looks to also be the
reason of there now being "GEN    doc/asconfig.texi" on every
(incremental) rebuild. Would be nice if that could be avoided. How
important is it to actually retain the timestamp? Said commit said
nothing on the "Why" aspect ... The issue described in the bug doesn't
look like it wants dealing with by using "cp -p" (or alike).

Comments

Nick Clifton July 31, 2023, 2:29 p.m. UTC | #1 
Hi Jan,

> PR 28909
> 
> Sadly "cp -p", doing more than just preserving the time stamp, can fail
> e.g. upon trying to preserve ownership (which we don't care about), as
> can be observed on e.g. Cygwin. Replace the use of -p by a use of touch,
> this way also only preserving modification time.

Thanks for fixing this.  If you have not done so already, please also apply
the patch to the 2.41 branch.


> Interestingly that earlier commit (8034b0baeac1) looks to also be the
> reason of there now being "GEN    doc/asconfig.texi" on every
> (incremental) rebuild.

Strange, I thought that the point was to avoid this kind of thing.

> Would be nice if that could be avoided. 

Rebuilding does not take that long though, does it ?

> How
> important is it to actually retain the timestamp? Said commit said
> nothing on the "Why" aspect ... The issue described in the bug doesn't
> look like it wants dealing with by using "cp -p" (or alike).

Well as I understood it, the point was that using "cp -p" meant that the
timestamps on the doc/asconfig.tex and doc/asconfig.texi files would
always be the same, so the build system would not decide to rebuild the
documentation and hence there would be no problem if the makeinfo program
was not available.

Cheers
   Nick
Jan Beulich July 31, 2023, 2:52 p.m. UTC | #2 
On 31.07.2023 16:29, Nick Clifton wrote:
>> PR 28909
>>
>> Sadly "cp -p", doing more than just preserving the time stamp, can fail
>> e.g. upon trying to preserve ownership (which we don't care about), as
>> can be observed on e.g. Cygwin. Replace the use of -p by a use of touch,
>> this way also only preserving modification time.
> 
> Thanks for fixing this.  If you have not done so already, please also apply
> the patch to the 2.41 branch.

I will; I haven't pushed to master either, to first see if there are
comments (or if perhaps we want to undo the earlier change).

>> Interestingly that earlier commit (8034b0baeac1) looks to also be the
>> reason of there now being "GEN    doc/asconfig.texi" on every
>> (incremental) rebuild.
> 
> Strange, I thought that the point was to avoid this kind of thing.
> 
>> Would be nice if that could be avoided. 
> 
> Rebuilding does not take that long though, does it ?

No, it's quick. But the expectation from an incremental build is that
only files depending on ones which were touched would be rebuilt. Hence
me noticing that slight anomaly.

>> How
>> important is it to actually retain the timestamp? Said commit said
>> nothing on the "Why" aspect ... The issue described in the bug doesn't
>> look like it wants dealing with by using "cp -p" (or alike).
> 
> Well as I understood it, the point was that using "cp -p" meant that the
> timestamps on the doc/asconfig.tex and doc/asconfig.texi files would
> always be the same, so the build system would not decide to rebuild the
> documentation and hence there would be no problem if the makeinfo program
> was not available.

First of all I was under the impression that there are already
precautions for that case. But I may be wrong (in which cases it may be
worth trying to introduce such logic), or such logic may have bit-rotted.
(On Cygwin I have been using MAKEINFO=true on the make command line
virtually forever, to escape such issues. Until things broke earlier
today, when I tried building 2.41.) And then my experience with build
systems tells me that it is extremely rare that fiddling with time stamps
is an appropriate solution. But then I'm afraid I may not properly
understand the original issue, and hence this might be a case where doing
so is justified.

Jan
diff mbox series

Patch

--- a/gas/Makefile.in
+++ b/gas/Makefile.in
@@ -2230,7 +2230,7 @@  de-stage3:
 
 doc/asconfig.texi: doc/$(CONFIG).texi doc/$(am__dirstamp)
 	$(AM_V_at)rm -f doc/asconfig.texi
-	$(AM_V_GEN)cp -p $(srcdir)/doc/$(CONFIG).texi doc/asconfig.texi
+	$(AM_V_GEN)cp $(srcdir)/doc/$(CONFIG).texi doc/asconfig.texi && touch -m -r $(srcdir)/doc/$(CONFIG).texi doc/asconfig.texi
 	$(AM_V_at)chmod u+w doc/asconfig.texi
 
 # Maintenance
--- a/gas/doc/local.mk
+++ b/gas/doc/local.mk
@@ -41,7 +41,7 @@  TEXI2DVI = texi2dvi -I "$(srcdir)/%D%" -
 
 %D%/asconfig.texi: %D%/$(CONFIG).texi %D%/$(am__dirstamp)
 	$(AM_V_at)rm -f %D%/asconfig.texi
-	$(AM_V_GEN)cp -p $(srcdir)/%D%/$(CONFIG).texi %D%/asconfig.texi
+	$(AM_V_GEN)cp $(srcdir)/%D%/$(CONFIG).texi %D%/asconfig.texi && touch -m -r $(srcdir)/%D%/$(CONFIG).texi %D%/asconfig.texi
 	$(AM_V_at)chmod u+w %D%/asconfig.texi
 
 CPU_DOCS = \