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

[Collin Funk]

Hi Bruno,

On 3/15/24 6:09 PM, Bruno Haible wrote:
> To make these two options interfere with each other, so that the last one
> wins, how about using the same variable for both?

Well, now I feel silly...
Let me know how this looks. I've imported CopyAction to main and use
that in a similar way to what you wrote.

I've used the name copymode instead of copyaction to match
gnulib-tool.sh. This also means changing GLConfig to store the Enum
itself instead of booleans for symlink and hardlink.

> * GLConfig.py:
> 
>         self.resetLHardlink()
>         if lhardlink != None:
>             self.resetLHardlink()

Yes, this was incorrect.

> * GLFileSystem.py:
> 
>                     constants.hardlink(lookedup, joinpath(destdir, rewritten))
> 
>   Please import the 'hardlink' symbol, so that you can reference it like this:
> 
>                     hardlink(lookedup, joinpath(destdir, rewritten))

I didn't do this since the invocations of link_if_changed and substart
used the 'constants.' prefix. Since you wanted this one change I have
gone ahead and done the rest of them.

In main(), all of the classes are prefixed with 'classes.'. Because
there are many occurrences of it, I have stuck to that convention for
now.

In GLConfig.py, I have also used the 'classes.' for the CopyAction
Enum. I tried to get rid of this but I ran into recursive import
errors.

I rather submit another patch later fixing that. The import stuff is a
bit confusing to me and makes me miss #include very much. I rather not
make this patch harder to read.

> * GLTestDir.py line 369:
> 
>                 if self.filesystem.shouldLink(src, lookedup) == 
> CopyAction.Hardlink:
> 
>   Are you sure this shouldn't be an 'elif'?

It should be 'elif', good catch. In the shell script it was an
'if $condition' that was nested under an 'else'. My eyes skipped over
the else so I misinterpreted it. Thanks.

>   According to the TODO file, the patch should implement the options
>     -S | --more-symlinks
>     -H | --more-hardlinks
>   But it doesn't do so.

These are just aliases for the other options correct?

I've added them to the corresponding add_argument call like this:

     parser.add_argument('-h', '-H', '--hardlink', '--more-hardlinks',
                         ...)  # rest

And similarly for --symlink.

Collin

0001-gnulib-tool.py-Follow-gnulib-tool-changes-part-58.patch
Description: Text Data