{"thread":{"id":"59691","subject":"[PATCH] doc: doc-diff: specify date","startedAt":"2023-05-03T23:23:59Z","lastAt":"2023-05-08T02:08:43Z","messageCount":6,"participants":["Felipe Contreras","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"476580","messageId":"20230503232349.59997-1-felipe.contreras@gmail.com","threadId":"59691","inReplyTo":null,"subject":"[PATCH] doc: doc-diff: specify date","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-03T23:23:49Z","receivedAt":"2023-05-03T23:23:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Otherwise comparing the output of commits with different dates generates\nunnecessary diffs.\n\nSigned-off-by: Felipe Contreras \n---\n Documentation/doc-diff | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex 1694300e50..554a78a12d 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -153,6 +153,7 @@ render_tree () {\n \t\tmake -j$parallel -C \"$tmp/worktree\" \\\n \t\t\t$makemanflags \\\n \t\t\tGIT_VERSION=omitted \\\n+\t\t\tGIT_DATE=1970-01-01 \\\n \t\t\tSOURCE_DATE_EPOCH=0 \\\n \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n \t\t\tinstall-man &&\n-- \n2.40.0+fc1\n\n"},{"id":"476611","messageId":"xmqq8re3inn4.fsf@gitster.g","threadId":"59691","inReplyTo":"20230503232349.59997-1-felipe.contreras@gmail.com","subject":"Re: [PATCH] doc: doc-diff: specify date","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-05T01:15:59Z","receivedAt":"2023-05-05T01:18:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras writes:\n\n> Otherwise comparing the output of commits with different dates generates\n> unnecessary diffs.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> Documentation/doc-diff | 1 +\n> 1 file changed, 1 insertion(+)\n\nAhh, it is a fix for a fallout from 28fde3a1 (doc: set actual\nrevdate for manpages, 2023-04-13); when it is shown in the patch\nform like this, it is kind of obvious why we need to compensate for\nthat change this way, but apparently \"doc-diff\" slipped everybody's\nmind back then when we were looking at the change.\n\nLooking at the patch text of 28fde3a1, we pass GIT_VERSION and\nGIT_DATE to AsciiDoc since that version. We were already covering\nGIT_VERSION by hardcoded \"omitted\" string, and now we compensate for\nthe other one here, which means this change and the other changes\ncomplement each other, and there shouldn't be a need to further\nadjustment for that change around this area. Looking good.\n\n> diff --git a/Documentation/doc-diff b/Documentation/doc-diff\n> index 1694300e50..554a78a12d 100755\n> --- a/Documentation/doc-diff\n> +++ b/Documentation/doc-diff\n> @@ -153,6 +153,7 @@ render_tree () {\n> \t\tmake -j$parallel -C \"$tmp/worktree\" \\\n> \t\t\t$makemanflags \\\n> \t\t\tGIT_VERSION=omitted \\\n> +\t\t\tGIT_DATE=1970-01-01 \\\n> \t\t\tSOURCE_DATE_EPOCH=0 \\\n> \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n> \t\t\tinstall-man &&\n\nI wonder what the existing SOURCE_DATE_EPOCH was trying to do there,\nthough.\n\nWill queue. Thanks.\n"},{"id":"476613","messageId":"20230505014610.GA2366370@coredump.intra.peff.net","threadId":"59691","inReplyTo":"xmqq8re3inn4.fsf@gitster.g","subject":"Re: [PATCH] doc: doc-diff: specify date","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-05T01:46:10Z","receivedAt":"2023-05-05T01:46:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 04, 2023 at 06:15:59PM -0700, Junio C Hamano wrote:\n\n> > diff --git a/Documentation/doc-diff b/Documentation/doc-diff\n> > index 1694300e50..554a78a12d 100755\n> > --- a/Documentation/doc-diff\n> > +++ b/Documentation/doc-diff\n> > @@ -153,6 +153,7 @@ render_tree () {\n> > \t\tmake -j$parallel -C \"$tmp/worktree\" \\\n> > \t\t\t$makemanflags \\\n> > \t\t\tGIT_VERSION=omitted \\\n> > +\t\t\tGIT_DATE=1970-01-01 \\\n> > \t\t\tSOURCE_DATE_EPOCH=0 \\\n> > \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n> > \t\t\tinstall-man &&\n> \n> I wonder what the existing SOURCE_DATE_EPOCH was trying to do there,\n> though.\n\nIt used to be necessary so that we had a reproducible build. Otherwise,\nasciidoc uses the mtime of the file, and diffing two versions would have\ntons of uninteresting date-differences.\n\nAfter 28fde3a1 I doubt it is necessary, as the header uses $GIT_DATE\ninstead (it's possible the mtime may be used elsewhere, but I didn't see\nany spot after grepping a built xml file. And at any rate, if it does\nnot produce a visible difference, that is enough for doc-diff).\n\n-Peff\n"},{"id":"476616","messageId":"xmqqzg6jgw47.fsf@gitster.g","threadId":"59691","inReplyTo":"20230505014610.GA2366370@coredump.intra.peff.net","subject":"Re: [PATCH] doc: doc-diff: specify date","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-05T05:55:52Z","receivedAt":"2023-05-05T05:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> > \t\t\tGIT_VERSION=omitted \\\n>> > +\t\t\tGIT_DATE=1970-01-01 \\\n>> > \t\t\tSOURCE_DATE_EPOCH=0 \\\n>> > \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n>> > \t\t\tinstall-man &&\n>> \n>> I wonder what the existing SOURCE_DATE_EPOCH was trying to do there,\n>> though.\n>\n> It used to be necessary so that we had a reproducible build. Otherwise,\n> asciidoc uses the mtime of the file, and diffing two versions would have\n> tons of uninteresting date-differences.\n>\n> After 28fde3a1 I doubt it is necessary, as the header uses $GIT_DATE\n> instead (it's possible the mtime may be used elsewhere, but I didn't see\n> any spot after grepping a built xml file. And at any rate, if it does\n> not produce a visible difference, that is enough for doc-diff).\n\nThanks for confirming my suspicion. I guess leaving it there still\nwould not hurt. It can be removed whenever somebody motivated\nenough comes and shows a well-reasoned patch that explains why it no\nlonger is necessary ;-)\n\n"},{"id":"476648","messageId":"20230505211610.GA3197168@coredump.intra.peff.net","threadId":"59691","inReplyTo":"xmqqzg6jgw47.fsf@gitster.g","subject":"Re: [PATCH] doc: doc-diff: specify date","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-05-05T21:16:10Z","receivedAt":"2023-05-05T21:16:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 04, 2023 at 10:55:52PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> >> > \t\t\tGIT_VERSION=omitted \\\n> >> > +\t\t\tGIT_DATE=1970-01-01 \\\n> >> > \t\t\tSOURCE_DATE_EPOCH=0 \\\n> >> > \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n> >> > \t\t\tinstall-man &&\n> >> \n> >> I wonder what the existing SOURCE_DATE_EPOCH was trying to do there,\n> >> though.\n> >\n> > It used to be necessary so that we had a reproducible build. Otherwise,\n> > asciidoc uses the mtime of the file, and diffing two versions would have\n> > tons of uninteresting date-differences.\n> >\n> > After 28fde3a1 I doubt it is necessary, as the header uses $GIT_DATE\n> > instead (it's possible the mtime may be used elsewhere, but I didn't see\n> > any spot after grepping a built xml file. And at any rate, if it does\n> > not produce a visible difference, that is enough for doc-diff).\n> \n> Thanks for confirming my suspicion. I guess leaving it there still\n> would not hurt. It can be removed whenever somebody motivated\n> enough comes and shows a well-reasoned patch that explains why it no\n> longer is necessary ;-)\n\n-- >8 --\nSubject: [PATCH] doc-diff: drop SOURCE_DATE_EPOCH override\n\nThe original doc-diff script set SOURCE_DATE_EPOCH to make asciidoc's\noutput deterministic. Otherwise, the mtime of the source files would end\nup in the footer of the manpage, causing noisy and uninteresting diff\nhunks.\n\nBut this has been unused since 28fde3a1f4 (doc: set actual revdate for\nmanpages, 2023-04-13), as the footer uses the externally-specified\nGIT_DATE instead (that needs to be set consistently, too, which it now\nis as of the previous commit).\n\nAsciidoc sets several automatic attributes based on the mtime (or manual\nepoch), so it's still possible to write a document that would need\nSOURCE_DATE_EPOCH set to be deterministic. But if we wrote such a thing,\nit's probably a mistake, and we're better off having doc-diff loudly\nshow it.\n\nSigned-off-by: Jeff King \n---\n Documentation/doc-diff | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/Documentation/doc-diff b/Documentation/doc-diff\nindex 554a78a12d..fb09e0ac0e 100755\n--- a/Documentation/doc-diff\n+++ b/Documentation/doc-diff\n@@ -154,7 +154,6 @@ render_tree () {\n \t\t\t$makemanflags \\\n \t\t\tGIT_VERSION=omitted \\\n \t\t\tGIT_DATE=1970-01-01 \\\n-\t\t\tSOURCE_DATE_EPOCH=0 \\\n \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n \t\t\tinstall-man &&\n \t\tmv \"$tmp/installed/$dname+\" \"$tmp/installed/$dname\"\n-- \n2.40.1.802.gdef2a8734a\n\n"},{"id":"476737","messageId":"645859a541f23_4e612944b@chronos.notmuch","threadId":"59691","inReplyTo":"xmqq8re3inn4.fsf@gitster.g","subject":"Re: [PATCH] doc: doc-diff: specify date","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-08T02:08:37Z","receivedAt":"2023-05-08T02:08:43Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Junio C Hamano wrote:\n> Felipe Contreras writes:\n> \n> > Otherwise comparing the output of commits with different dates generates\n> > unnecessary diffs.\n> >\n> > Signed-off-by: Felipe Contreras \n> > ---\n> > Documentation/doc-diff | 1 +\n> > 1 file changed, 1 insertion(+)\n> \n> Ahh, it is a fix for a fallout from 28fde3a1 (doc: set actual\n> revdate for manpages, 2023-04-13); when it is shown in the patch\n> form like this, it is kind of obvious why we need to compensate for\n> that change this way, but apparently \"doc-diff\" slipped everybody's\n> mind back then when we were looking at the change.\n\nYes. doc-diff is an odd duck, because it can't be easily integrated to\nthe testing framework.\n\nSometimes a diff in the documentation is intentional, so the fact that\ndoc-diff generates an output from HEAD~ to HEAD is precisely what was\nintended. However, sometimes it's not. Maybe a flag inside the commit\nmessage such as GitHub's `[no ci]` might help, but it's beyond me how\ncould that be cleanly integrated to continous integration machinery.\n\nFor now doc-diff is meant to be run manually, therefore it's expected\nthat some unexpeced diffs are inevitably going to slip by, and more\nrelevantly: issues in doc-diff itself are going to slip by.\n\n> Looking at the patch text of 28fde3a1, we pass GIT_VERSION and\n> GIT_DATE to AsciiDoc since that version. We were already covering\n> GIT_VERSION by hardcoded \"omitted\" string, and now we compensate for\n> the other one here, which means this change and the other changes\n> complement each other, and there shouldn't be a need to further\n> adjustment for that change around this area. Looking good.\n\nYes. I think we should be passing a semi-real version instead, like\n`0.0.0`, just to see how a real version would look like, but that's\northogonal.\n\n> > diff --git a/Documentation/doc-diff b/Documentation/doc-diff\n> > index 1694300e50..554a78a12d 100755\n> > --- a/Documentation/doc-diff\n> > +++ b/Documentation/doc-diff\n> > @@ -153,6 +153,7 @@ render_tree () {\n> > \t\tmake -j$parallel -C \"$tmp/worktree\" \\\n> > \t\t\t$makemanflags \\\n> > \t\t\tGIT_VERSION=omitted \\\n> > +\t\t\tGIT_DATE=1970-01-01 \\\n> > \t\t\tSOURCE_DATE_EPOCH=0 \\\n> > \t\t\tDESTDIR=\"$tmp/installed/$dname+\" \\\n> > \t\t\tinstall-man &&\n> \n> I wonder what the existing SOURCE_DATE_EPOCH was trying to do there,\n> though.\n\nI also wondered the same, but again: orthogonal.\n\nCheers.\n\n-- \nFelipe Contreras\n"}]}