bug-gnulib@gnu.org mirror (unofficial)
 help / color / mirror / Atom feed
From: Collin Funk <collin.funk1@gmail.com>
To: Bruno Haible <bruno@clisp.org>, bug-gnulib@gnu.org
Subject: Re: gnulib-tool.py: Follow gnulib-tool changes, part 28.
Date: Sat, 24 Feb 2024 19:32:07 -0800	[thread overview]
Message-ID: <0e0bf52d-f9c3-4cde-a1e0-2701be639bfc@gmail.com> (raw)
In-Reply-To: <4683259.eGJsNajkDb@nimes>

[-- Attachment #1: Type: text/plain, Size: 1399 bytes --]

Hi Bruno,

On 2/24/24 5:25 PM, Bruno Haible wrote:
> None of the 'sort' invocations in gnulib-tool are covered by an
> entry in the gnulib-tool.py.TODO file. Therefore the most promising
> approach to finding the cause of the difference is to
>   - go through all 'sort' invocations in gnulib-tool,
>   - find the corresponding place in pygnulib/ and see whether
>     sorting happens there as well or has been forgotten.

Thank you for the advice and for looking over all of these patches. I
think that this patch should be a solution to the issue as long as I
am understanding gnulib-tool correctly.

From what I could understand, it seems that upon seeing that the mode
is "import", the specified_modules is set with the unsorted list of
modules [1]. Then that variable is sorted before it is used [2]. Could
you confirm that I am understanding this correctly?

If so, I feel that this patch should be the correct way to handle it.
My initial ideas where to sort the modules in main() before the
GLConfig object was created, or to modify the GLConfig class's methods
which are used to set the modules in use. I think the second option is
less likely to be broken by future changes. The changes involve using
a set and sorted() to substitute `sort -u'.

[1] https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool#n5085
[2] https://git.savannah.gnu.org/cgit/gnulib.git/tree/gnulib-tool#n5268

[-- Attachment #2: 0001-gnulib-tool.py-Handle-the-sorting-of-modules-correct.patch --]
[-- Type: text/x-patch, Size: 3316 bytes --]

From 93d4e7698afc92e3b257c2a0b2050540357edc96 Mon Sep 17 00:00:00 2001
From: Collin Funk <collin.funk1@gmail.com>
Date: Sat, 24 Feb 2024 19:05:51 -0800
Subject: [PATCH] gnulib-tool.py: Handle the sorting of modules correctly.

See discussion at
<https://lists.gnu.org/archive/html/bug-gnulib/2024-02/msg00219.html>

* pygnulib/GLConfig.py (GLConfig.addModule, CLConfig.setModules): Use a
sorted set when adding modules to replicate "sort -u" used by
gnulib-tool.
* pygnulib/GLImport.py (GLImport.actioncmd): Don't sort modules when
constructing actioncmd as GLConfig handles it for us.
---
 ChangeLog            | 11 +++++++++++
 pygnulib/GLConfig.py |  6 ++++--
 pygnulib/GLImport.py |  4 ++--
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 1910933eb7..a616db7e09 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2024-02-24  Collin Funk  <collin.funk1@gmail.com>
+
+	gnulib-tool.py: Handle the sorting of modules correctly.
+	See discussion at
+	<https://lists.gnu.org/archive/html/bug-gnulib/2024-02/msg00219.html>
+	* pygnulib/GLConfig.py (GLConfig.addModule, CLConfig.setModules): Use a
+	sorted set when adding modules to replicate "sort -u" used by
+	gnulib-tool.
+	* pygnulib/GLImport.py (GLImport.actioncmd): Don't sort modules when
+	constructing actioncmd as GLConfig handles it for us.
+
 2024-02-24  Collin Funk  <collin.funk1@gmail.com>
 
 	gnulib-tool.py: Follow gnulib-tool changes, part 28.
diff --git a/pygnulib/GLConfig.py b/pygnulib/GLConfig.py
index bdebf243cc..03da6f8e3c 100644
--- a/pygnulib/GLConfig.py
+++ b/pygnulib/GLConfig.py
@@ -476,7 +476,7 @@ class GLConfig(object):
         '''Add the module to the modules list.'''
         if type(module) is str:
             if module not in self.table['modules']:
-                self.table['modules'] += [module]
+                self.table['modules'] = sorted(list(set(self.table['modules'] + [module])))
         else:  # if module has not str type
             raise TypeError('module must be a string, not %s'
                             % type(module).__name__)
@@ -499,7 +499,9 @@ class GLConfig(object):
         if type(modules) is list or type(modules) is tuple:
             old_modules = self.table['modules']
             self.table['modules'] = list()
-            for module in modules:
+            # Canonicalize the list of specified modules.
+            # sort -u
+            for module in sorted(set(modules)):
                 try:  # Try to add each module
                     self.addModule(module)
                 except TypeError as error:
diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py
index c69a33deb7..9b08e7e4b6 100644
--- a/pygnulib/GLImport.py
+++ b/pygnulib/GLImport.py
@@ -438,9 +438,9 @@ class GLImport(object):
         elif vc_files == False:
             actioncmd += ' \\\n#  --no-vc-files'
         if len(avoids) > 0:
-            actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in sorted(avoids)])
+            actioncmd += ''.join([f" \\\n#  --avoid={x}" for x in avoids])
         if len(modules) > 0:
-            actioncmd += ''.join([f" \\\n#  {x}" for x in sorted(modules)])
+            actioncmd += ''.join([f" \\\n#  {x}" for x in modules])
         return actioncmd
 
     def relative_to_destdir(self, dir):
-- 
2.39.2


  reply	other threads:[~2024-02-25  3:32 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-23  5:23 [PATCH] gnulib-tool.py: Follow gnulib-tool changes, part 27 Collin Funk
2024-02-23 13:08 ` Bruno Haible
2024-02-23 22:20   ` Collin Funk
2024-02-23 23:51     ` Bruno Haible
2024-02-24  2:36       ` Collin Funk
2024-02-24  5:49         ` gnulib-tool.py: Follow gnulib-tool changes, part 28 Collin Funk
2024-02-24 23:25           ` gnulib-tool.py: Follow gnulib-tool changes, part 27 Bruno Haible
2024-02-25  0:03             ` Dima Pasechnik
2024-02-25 11:57               ` Python != None Bruno Haible
2024-02-25 19:29                 ` Collin Funk
2024-02-25 20:07                   ` Collin Funk
2024-02-26 20:38                     ` pycodestyle configuration Bruno Haible
2024-02-26 21:31                       ` Collin Funk
2024-02-26 22:54                         ` Bruno Haible
2024-02-27  0:51                           ` Collin Funk
2024-02-27  2:38                             ` Bruno Haible
2024-02-27  4:22                               ` Collin Funk
2024-02-25 20:55                   ` Python != None Dima Pasechnik
2024-02-25 12:02             ` Python 'strings' Bruno Haible
2024-02-25 19:05               ` Collin Funk
2024-02-24 23:42           ` gnulib-tool.py: Follow gnulib-tool changes, part 28 Bruno Haible
2024-02-25  0:47             ` Collin Funk
2024-02-25  1:18               ` Collin Funk
2024-02-25  1:25                 ` Bruno Haible
2024-02-25  3:32                   ` Collin Funk [this message]
2024-02-26 20:51                     ` Bruno Haible
2024-02-28 11:51                       ` Collin Funk
2024-02-28 12:14                         ` Bruno Haible

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.gnu.org/mailman/listinfo/bug-gnulib

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0e0bf52d-f9c3-4cde-a1e0-2701be639bfc@gmail.com \
    --to=collin.funk1@gmail.com \
    --cc=bruno@clisp.org \
    --cc=bug-gnulib@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).