bug-gnulib
[Top][All Lists]
Advanced

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

Re: [PATCH] gnulib-tool.py: Store the module name instead of computing i


From: Bruno Haible
Subject: Re: [PATCH] gnulib-tool.py: Store the module name instead of computing it.
Date: Tue, 26 Mar 2024 13:20:46 +0100

Hi Collin,

> But, atleast we are not back to taking 20 seconds to run ./test-all.sh.

It's good that you are looking at the execution times. These __eq__
etc. methods are apparently heavily used.

> It seems that Python has an index() function to find the first
> occurrence of an item in a list, but they don't have one for the last
> occurrence [1]. Do other languages have this? Maybe we can define a
> new one.

If someone needs the last occurrence of an item in a list, there is
some suspicion that they are using a wrong data representation. Maybe
they need a reversed list?

> We have to find the last occurrence though just incase a project using
> Gnulib is in a 'modules' subdirectory. In that case finding the first
> would break things.
> 
> The other important thing is the unpacking on this line [2]:
> 
>     self.name = str(Path(*parts[name_index + 1:]))
> 
> This is important because some of the modules, like cryptography ones,
> are in subdirectories. Therefore, we have to combine all components
> following the last 'modules/' directory.

Ah, now I see what you are aiming at.

These are two different, orthogonal topics:
  - Caching the 'name' of a module, to reduce the execution times.
  - Fixing a rarely occurring problem when "a project using
    Gnulib is in a 'modules' subdirectory".
Since they are orthogonal, please submit separate patches. This simplifies
not only the code review, but also your own thinking.

Caching the 'name' instead of the 'path' is fine. Go ahead with that.

Regarding the 'modules' subdirectory: When I see you replacing 2 lines of code
with 8 lines of code, and especially searching for a 'modules' file name
component, I wonder whether there's no simpler solution.
Namely, a module of name NAME can be located at one of the following locations:
  gnulib_dir/modules/NAME
  local_dir1/modules/NAME
  local_dir2/modules/NAME
  ...
(Or it can logically be in such a location but physically reside in a temporary
file.)
So, if the GLModule.__init__ method knows about which directory (gnulib_dir,
local_dir1, local_dir2, ...) resolved the lookup from GLModuleSystem.py:108,
it just needs to remove the /modules/ file name component from that (fixed!)
position. I think this should be simpler than to search for a /modules/ file
name component and then argue whether the first one found or the last one
found is the better match.

Bruno






reply via email to

[Prev in Thread] Current Thread [Next in Thread]