Hi Collin,
> This patch implements --hardlink and --local-hardlink.
Thanks; that's major (and tricky), indeed.
> Notice the arguments '-s --hardlink --symlink -h'. I tried to make
> sure gnulib-tool.py behaved similarly to gnulib-tool.sh in this case.
>
> I beleive the correct behavior is to use the last option given so
> '-h'. ...
Yes.
> It doesn't seem there is a good way to do this with Python's argparse,
> though I may not have looked at the documentation hard enough [1].
Well, when the code contains
parser.add_argument('-s', '--symbolic', '--symlink',
dest='symlink',
default=None,
action='store_true')
...
parser.add_argument('-h', '--hardlink',
dest='hardlink',
default=None,
action='store_true')
each of -s and -h will store into a different variable. So, they will
act independently.
To make these two options interfere with each other, so that the last one
wins, how about using the same variable for both?
class CopyAction(Enum):
Copy = 0
Symlink = 1
Hardlink = 2
parser.add_argument('-s', '--symbolic', '--symlink',
dest='copyaction',
default=None,
action='store_const', const=CopyAction.Symlink)
...
parser.add_argument('-h', '--hardlink',
dest='copyaction',
default=None,
action='store_const', const=CopyAction.Hardlink)
> I ended up doing this which seems to work, though is not very pretty:
>
> # --symlink and --hardlink are mutually exclusive.
> # Use the last one given.
> if symlink and hardlink:
> optindex_table = {}
> options = list(reversed(sys.argv[1:]))
> options_count = len(options)
> for option in ['-s', '--symbolic', '--symlink', '-h', '--hardlink']:
> if option in options:
> optindex_table[option] = options_count - options.index(option)
> last_option = max(optindex_table, key=optindex_table.get)
> # Disable the option that is not the last one.
> if last_option in ['-s', '--symbolic', '--symlink']:
> hardlink = False
> else: # last_option is --hardlink or equivalent.
> symlink = False
>
> And something similar for the --local-* variants.
Yes, this is not pretty. I wish you can remove this code, if the outline
above works.
Other than that:
* GLConfig.py:
self.resetLHardlink()
if lhardlink != None:
self.resetLHardlink()
The last of these 3 lines looks wrong.
* 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))
* GLTestDir.py line 369:
if self.filesystem.shouldLink(src, lookedup) ==
CopyAction.Hardlink:
Are you sure this shouldn't be an 'elif'?
* main.py:
According to the TODO file, the patch should implement the options
-S | --more-symlinks
-H | --more-hardlinks
But it doesn't do so.
Bruno
