Re: [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 27.

[Bruno Haible <bruno@clisp.org>]

Hello Collin,

Collin Funk wrote:
> Hello, here is a patch implementing the --gnu-make option for
> gnulib-tool.py.

Thanks! Applied. This was already a major piece of work.

> All of these
> commits were grouped together and were similar so I felt like it
> didn't make too much sense to handle them separately.

Yes, keeping them together was the right thing to do.

> I ended up testing these changes with Paul Eggert's merge-gnulib
> script used by Emacs [1]. Maybe there is a better way to test it but
> this seemed to work pretty well. First I would update the sources with
> the regular gnulib-tool, save the generated Makefile, and then run the
> script again with gnulib-tool.py.

Perfect. That's the perfect way to test it this --gnu-make option.

Just three small nits that you might want to revisit:

* gnulib-tool.py line 610:
+    if modules != None and "tests" in mode and gnu_make:
        ^^^^^^^^^^^^^^^^^^^
What is the rationale for this condition? It is not present in the original. Can
it be removed?

* pygnulib/GLEmiter.py lines 732, 739:
+                        allsnippets += re.sub(r'^if (.*)', r'ifneq (,$(\1))', amsnippet1)
+                        allsnippets += re.sub(r'^if (.*)', r'ifneq (,$(\1))', amsnippet2)

The regular expression is duplicated. The problem with duplicated code is
that it very often leads to bugs in the future: Future code maintainers
will see one copy of the piece of code, modify it, and leave the other
copy unchanged. Sometimes it's a bug because it's inconsistent; sometimes it's
a bug because the other copy lacks the feature for which the modification
was made in the first copy.

So, the lesson is: Try hard to avoid code duplication!

* pygnulib/GLConfig.py line 802:
Typo: Autmake -> Automake.


Bruno