diff mbox series

[bug#70169,v2,09/12] maint: Remove %%CreationDate from generated EPS files

Message ID 109c05ead54c3e48a8df27794c8df56149343a37.1712437365.git.janneke@gnu.org
State New
Headers show
Series Reproducible `make dist' tarball in defiance of Autotools and Gettext | expand

Commit Message

Janneke Nieuwenhuizen April 6, 2024, 9:18 p.m. UTC 
* doc/local.mk (.dot.eps, png.eps): Remove %%CreationDate.  Split single shell
command into separate recipe lines, prefixed by $(AM_V_at).

Change-Id: I5a03485c19c72f0c46411815c51290e52a8e5399
---
 doc/local.mk | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Ludovic Courtès April 8, 2024, 9:41 a.m. UTC | #1 
Janneke Nieuwenhuizen <janneke@gnu.org> skribis:

> * doc/local.mk (.dot.eps, png.eps): Remove %%CreationDate.  Split single shell
> command into separate recipe lines, prefixed by $(AM_V_at).
>
> Change-Id: I5a03485c19c72f0c46411815c51290e52a8e5399
> ---
>  doc/local.mk | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/doc/local.mk b/doc/local.mk
> index c355bec8d7..60e36091ce 100644
> --- a/doc/local.mk
> +++ b/doc/local.mk
> @@ -152,12 +152,16 @@ DOT_OPTIONS =					\
>  	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
>  
>  .dot.eps:
> -	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"; \
> -	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
> +	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"
> +	$(AM_V_at)grep -v ^%%CreationDate: "$(srcdir)/$@.tmp" > "$(srcdir)/$@.tmp2"
> +	$(AM_V_at)rm -f "$(srcdir)/$@.tmp"
> +	$(AM_V_at)mv "$(srcdir)/$@.tmp2" "$@"

Apparently ‘dot’ (GraphViz) does not emit a ‘CreationDate’ header.

Should we drop this change and add a line like this above the ‘mv’ line:

  @if grep -q CreationDate "$@.tmp"; then false; else true; fi

to be on the safe side?

>  .png.eps:
> -	$(AM_V_GEN)convert "$<" "$@-tmp.eps"; \
> -	mv "$@-tmp.eps" "$@"
> +	$(AM_V_GEN)convert "$<" "$@-tmp.eps"
> +	$(AM_V_at)grep -v ^%%CreationDate: "$@-tmp.eps" > "$@-tmp2.eps"
> +	$(AM_V_at)rm -f "$@-tmp.eps"
> +	$(AM_V_at)mv "$@-tmp2.eps" "$@"

Looking into the code of ImageMagick, I see:

--8<---------------cut here---------------start------------->8---
        timer=GetMagickTime();
        (void) FormatMagickTime(timer,MaxTextExtent,date);
        (void) FormatLocaleString(buffer,MaxTextExtent,
          "%%%%CreationDate: %s\n",date);
--8<---------------cut here---------------end--------------->8---

where ‘GetMagickTime’ honors ‘SOURCE_DATE_EPOCH’.  Should we set
‘SOURCE_DATE_EPOCH’ and avoid the ‘grep’ trick?

OTOH, an argument to keep this patch as-is is that it’ll resist to
changes in ImageMagick/GraphViz.  So maybe the comments above aren’t
that relevant.

Ludo’.
Janneke Nieuwenhuizen April 8, 2024, 5:12 p.m. UTC | #2 
Ludovic Courtès writes:

> Janneke Nieuwenhuizen <janneke@gnu.org> skribis:
>
>> * doc/local.mk (.dot.eps, png.eps): Remove %%CreationDate.  Split single shell
>> command into separate recipe lines, prefixed by $(AM_V_at).
>>
>> Change-Id: I5a03485c19c72f0c46411815c51290e52a8e5399
>> ---
>>  doc/local.mk | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/doc/local.mk b/doc/local.mk
>> index c355bec8d7..60e36091ce 100644
>> --- a/doc/local.mk
>> +++ b/doc/local.mk
>> @@ -152,12 +152,16 @@ DOT_OPTIONS =					\
>>  	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
>>  
>>  .dot.eps:
>> -	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"; \
>> -	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
>> +	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"
>> +	$(AM_V_at)grep -v ^%%CreationDate: "$(srcdir)/$@.tmp" > "$(srcdir)/$@.tmp2"
>> +	$(AM_V_at)rm -f "$(srcdir)/$@.tmp"
>> +	$(AM_V_at)mv "$(srcdir)/$@.tmp2" "$@"
>
> Apparently ‘dot’ (GraphViz) does not emit a ‘CreationDate’ header.

Ah, good catch.

> Should we drop this change and add a line like this above the ‘mv’ line:

Probably I just copied the grep to remove it without much looking or
thinking :)

>   @if grep -q CreationDate "$@.tmp"; then false; else true; fi
>
> to be on the safe side?

Let's do that, I'm adding it as

    $(AM_V_at)! grep -q %%CreationDate "$(srcdir)/$@.tmp"

in v3.

>>  .png.eps:
>> -	$(AM_V_GEN)convert "$<" "$@-tmp.eps"; \
>> -	mv "$@-tmp.eps" "$@"
>> +	$(AM_V_GEN)convert "$<" "$@-tmp.eps"
>> +	$(AM_V_at)grep -v ^%%CreationDate: "$@-tmp.eps" > "$@-tmp2.eps"
>> +	$(AM_V_at)rm -f "$@-tmp.eps"
>> +	$(AM_V_at)mv "$@-tmp2.eps" "$@"
>
> Looking into the code of ImageMagick, I see:
>
>         timer=GetMagickTime();
>         (void) FormatMagickTime(timer,MaxTextExtent,date);
>         (void) FormatLocaleString(buffer,MaxTextExtent,
>           "%%%%CreationDate: %s\n",date);
>
> where ‘GetMagickTime’ honors ‘SOURCE_DATE_EPOCH’.  Should we set
> ‘SOURCE_DATE_EPOCH’ and avoid the ‘grep’ trick?

Good catch; and it already works.

> OTOH, an argument to keep this patch as-is is that it’ll resist to
> changes in ImageMagick/GraphViz.  So maybe the comments above aren’t
> that relevant.

Well, I like what we have now better; it's a bit sharper/cleaner anyway.

Thanks!
Janneke
diff mbox series

Patch

diff --git a/doc/local.mk b/doc/local.mk
index c355bec8d7..60e36091ce 100644
--- a/doc/local.mk
+++ b/doc/local.mk
@@ -152,12 +152,16 @@  DOT_OPTIONS =					\
 	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
 
 .dot.eps:
-	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"; \
-	mv "$(srcdir)/$@.tmp" "$(srcdir)/$@"
+	$(AM_V_DOT)$(DOT) -Teps $(DOT_OPTIONS) < "$<" > "$(srcdir)/$@.tmp"
+	$(AM_V_at)grep -v ^%%CreationDate: "$(srcdir)/$@.tmp" > "$(srcdir)/$@.tmp2"
+	$(AM_V_at)rm -f "$(srcdir)/$@.tmp"
+	$(AM_V_at)mv "$(srcdir)/$@.tmp2" "$@"
 
 .png.eps:
-	$(AM_V_GEN)convert "$<" "$@-tmp.eps"; \
-	mv "$@-tmp.eps" "$@"
+	$(AM_V_GEN)convert "$<" "$@-tmp.eps"
+	$(AM_V_at)grep -v ^%%CreationDate: "$@-tmp.eps" > "$@-tmp2.eps"
+	$(AM_V_at)rm -f "$@-tmp.eps"
+	$(AM_V_at)mv "$@-tmp2.eps" "$@"
 
 # We cannot add new dependencies to `%D%/guix.pdf' & co. (info "(automake)
 # Extending").  Using the `-local' rules is imperfect, because they may be