Re: [PATCH v5 4/6] Makefile: update to new command-list.txt format

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, May 9, 2015 at 1:17 PM, Sébastien Guimmara
<sebastien.guimmara@xxxxxxxxx> wrote:
> * In target common-cmds.h:
>   The AWK script 'generate-cmdlist.awk' replaces 'generate-cmdlist.sh'
>
> * In target check-docs:
>   command-list.txt now contains common commands group in
>   the header [common]. sed ignore all lines in command-list.txt
>   until the [commands] list to correctly checks for missing
>   documentation on Git commands.
>
> For the target common-cmds.h part:
> Signed-off-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>

The Makefile modification from my generate-cmdlist patch[1] is part of
the overall logical change of that patch. Its relation to the Makefile
changes in this patch is weak at best, or entirely non-existent.
Consequently, it should not be mixed with them. See my reply[2] to the
v5 cover letter for more information.

More below.

[1]: http://article.gmane.org/gmane.comp.version-control.git/268598
[2]: http://article.gmane.org/gmane.comp.version-control.git/268756

> For the target check-docs part:
> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
> Signed-off-by: Sébastien Guimmara <sebastien.guimmara@xxxxxxxxx>
> ---
> diff --git a/Makefile b/Makefile
> index 5f3987f..9f333e9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1687,10 +1687,10 @@ $(BUILT_INS): git$X
>         ln -s $< $@ 2>/dev/null || \
>         cp $< $@
>
> -common-cmds.h: ./generate-cmdlist.sh command-list.txt
> +common-cmds.h: generate-cmdlist.awk command-list.txt
>
>  common-cmds.h: $(wildcard Documentation/git-*.txt)
> -       $(QUIET_GEN)./generate-cmdlist.sh > $@+ && mv $@+ $@
> +       $(QUIET_GEN)awk -f generate-cmdlist.awk command-list.txt > $@+ && mv $@+ $@
>
>  SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
>         $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
> @@ -2447,7 +2447,7 @@ check-docs::
>                 esac ; \
>                 test -f "Documentation/$$v.txt" || \
>                 echo "no doc: $$v"; \
> -               sed -e '/^#/d' command-list.txt | \
> +               sed -e '1,/^\[commands\]/d' <command-list.txt | \

I'm not convinced that it's a good idea to drop comment-line
processing from this sed invocation. Even though the current
command-list.txt may not have any comments following the [commands]
header, there is no guarantee that someone won't some day add some
comments following the header.

sed accepts multiple -e arguments, so retaining comment-line
processing does not make the extraction any more expensive. For
instance:

    sed -e '1,/^\[commands\]/d' -e '/^#/d' <command-list.txt | \

>                 grep -q "^$$v[  ]" || \
>                 case "$$v" in \
>                 git) ;; \
> @@ -2455,7 +2455,7 @@ check-docs::
>                 esac ; \
>         done; \
>         ( \
> -               sed -e '/^#/d' \
> +               sed -e '1,/^\[commands\]/d' \

Ditto. It would be more robust to retain comment-line processing.

>                     -e 's/[     ].*//' \
>                     -e 's/^/listed /' command-list.txt; \
>                 $(MAKE) -C Documentation print-man1 | \
> --
> 2.4.0
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]