Re: [Quilt-dev] [PATCH 1/3] Avoid unnecessary rebuilds of the manpage and README

[Jean Delvare]

Hi Michal,

Le Monday 01 June 2015 à 13:08 +0200, Michal Marek a écrit :
> Only rebuild the files if any of the help texts changed. The speed
> improvement is not noticeable, but it makes the make output tidier when
> working on the scripts.

Makes sense.

> Signed-off-by: Michal Marek <address@hidden>
> ---
>  .gitignore  | 1 +
>  Makefile.in | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 70109e9..26ce8af 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -24,6 +24,7 @@ doc/quilt-rus.log
>  doc/quilt-rus.pdf
>  doc/README
>  doc/reference
> +doc/reference.tmp
>  
>  po/*.mo
>  
> diff --git a/Makefile.in b/Makefile.in
> index 4af6757..d4bcb5d 100644
> --- a/Makefile.in
> +++ b/Makefile.in
> @@ -215,7 +215,7 @@ doc/quilt.1: doc/quilt.1.in doc/reference $(COMPAT)
>         esac;                                                         \
>       done < $< 2>&1 > $@
>  
> -doc/reference : bin/quilt $(QUILT:%=quilt/%) quilt/scripts/patchfns 
> quilt/scripts/utilfns $(COMPAT)
> +doc/reference.tmp : bin/quilt $(QUILT:%=quilt/%) quilt/scripts/patchfns 
> quilt/scripts/utilfns $(COMPAT)
>       @unset LANG LC_MESSAGES LC_CTYPE LC_ALL;                        \
>       for i in $(sort $(QUILT));                                      \
>       do                                                              \
> @@ -227,6 +227,11 @@ doc/reference : bin/quilt $(QUILT:%=quilt/%) 
> quilt/scripts/patchfns quilt/script
>              -e '/^$$/!s/^/  /'                                       \
>              -e 's/^  Usage: *//' > $@
>  
> +doc/reference : doc/reference.tmp

I don't think you need a separate target. doc/reference.tmp is not
useful per se and doesn't need to survive the "make" call.

> +     @if ! cmp -s $@ $<; then                                        \

I'd rather use "diff -q" for comparison, as is already done in the
revert, diff, pop and refresh commands. Now I see that we have one
occurrence of cmp in patchfns, but I'll replace it with "diff -q" for
consistency.

> +             @CP@ -f $< $@;                                          \
> +     fi
> +
>  bin/guards.1 : bin/guards
>       $(POD2MAN) $< > $@
>  

Would the following alternative implementation work for you?

---
 Makefile.in |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/Makefile.in
+++ b/Makefile.in
@@ -225,7 +225,11 @@ doc/reference : bin/quilt $(QUILT:%=quil
        done |                                                          \
        $(SED) -e 's/\$$EDITOR ([^)]*)/$$EDITOR/'                       \
               -e '/^$$/!s/^/  /'                                       \
-              -e 's/^  Usage: *//' > $@
+              -e 's/^  Usage: *//' > address@hidden
+       @if test ! -e $@ || ! @DIFF@ -q address@hidden $@;                      
\
+       then                                                            \
+               mv -f address@hidden $@;                                        
\
+       fi
 
 bin/guards.1 : bin/guards
        $(POD2MAN) $< > $@

It makes the temporary file non-persistent so we don't have to put it
in .gitignore, nor in $DIRT (which you had forgotten in your patch BTW.)

-- 
Jean Delvare
SUSE L3 Support