cmd/gitannex: Add the gitannex command #7654

dmcardle · 2024-02-29T02:00:28Z

This PR adds a new subcommand named "gitannex", aka "git-annex-remote-rclone-builtin" when invoked via a symlink.

This accomplishes milestone 1 from issue #7625: "minimal support for the external special remote protocol".

What is the purpose of this change?

To add a shim program that enables git-annex to interact with rclone remotes.

Was the change discussed in an issue or in the forum before?

Checklist

I have read the contribution guidelines.
I have added tests for all changes in this PR if appropriate.
I have added documentation for the changes if appropriate.
All commit messages are in house style.
I'm done, this Pull Request is ready for review :-)

dmcardle · 2024-03-04T14:30:16Z

@ncw This is ready for review!

First of all, would you be willing to accept this new command in contrib/ or would you prefer I publish as a separate project? I like it living in the rclone repo because it uses rclone as a library — and I get to abuse your CI :).

At first, I wanted to add it as a new subcommand, e.g rclone gitannex, but it turns out that git-annex is incapable of invoking a "special remote program" with arguments. Rather, it just executes git-annex-remote-${REMOTE_TYPE} by design. I considered making a wrapper shell script with the expected name, but that would not be cross-platform. Ultimately, I settled on this design, where we just have a separate binary living in contrib/.

I wouldn't ask you to include this command in releases because it would probably double the size. Also, it's only useful to a very small, technically inclined crowd who can probably figure out how to install it themselves.

Thanks!

contrib/gitannex/git-annex-remote-rclone-goyle/main.go

.gitignore

contrib/gitannex/gitannex.go

ncw

I've given the code a light skim :-) It looks very well written.

I think we should give some thought to whether we should make this an rclone command or not.

If we make it an rclone command then it will be built in and easier to deploy for the users (without having to download go). It will of course bloat the binary with stuff, but that is a bridge we crossed a long time ago!

If it isn't an rclone command then it should probably live in its own repository (say rclone/gitannex). If you like that idea I'd be happy to make that repository and give you r/w access to it.

Let me know what you think?

contrib/gitannex/git-annex-remote-rclone-goyle/main.go

contrib/gitannex/gitannex.go

dmcardle · 2024-03-10T01:53:02Z

Hey @ncw, this is ready for another look when you get a chance.

I factored it back into a subcommand (rclone gitannex). It now recognizes when rclone is invoked via a symlink (or whatever) named git-annex-remote-rclone-goyle.

Thanks!

dmcardle · 2024-03-11T21:48:01Z

Just made a few little improvements, like more consistent use of the messageParser type and more test coverage.

dmcardle · 2024-03-14T15:44:28Z

Hey @ncw, this is in good shape now. PTAL when you get a chance!

joeyh · 2024-03-22T15:09:34Z

WRT "rclone gitannex" vs "git-annex-remote-rclone-goyle" I would be open to considering doing something in git-annex to support the former without needing the latter. If that would be helpful enough to bother with. It might be as simple as a hardcoded special case.

dmcardle · 2024-03-22T15:24:34Z

WRT "rclone gitannex" vs "git-annex-remote-rclone-goyle" I would be open to considering doing something in git-annex to support the former without needing the latter. If that would be helpful enough to bother with. It might be as simple as a hardcoded special case.

That would be really useful, @joeyh! Although I would probably keep the symlink detection logic in this PR in order to support current releases of git-annex. I'd keep it around until a git-annex release with the new feature trickled down to Debian stable.

My taste would be to implement a more generic mechanism rather than adding a special case for rclone gitannex. What if externaltype could be repeated, so that git annex initremote MyRemote type=external externaltype=rclone externaltype=gitannex ... would cause git-annex to exec rclone with the additional gitannex arg?

yarikoptic · 2024-03-22T22:00:46Z

cmd/gitannex/run-git-annex-testremote.sh

+#
+#   * This system has an rclone remote configured named "garrgoyle-test-remote".
+#
+#   * If it uses rclone's "local" backend, /tmp/garrgoyle-test-remote exists.


best to avoid any assumptions or otherwise would require per location where you would intend to run this test to set it up somehow etc. What about the approach we have in git-annex-remote-rclone to use temporary HOME with all needed setup? checkout

https://github.com/git-annex-remote-rclone/git-annex-remote-rclone/blob/master/tests/all-in-one.sh#L22

which might also give you ideas on additional (it is never enough!!) testing etc.

@yarikoptic Very clever! In the interest of unblocking the PR, I'll punt on improving the test script for now, but this is the very next thing on my TODO list for the project. I really appreciate you taking a look at the PR.

Traveling ATM, can't promise speedy review over weekend, but would be happy to help in general.

yarikoptic · 2024-03-22T22:02:48Z

cmd/gitannex/gitannex.go

+)
+
+const commandName string = "gitannex"
+const garrgoyleName string = "git-annex-remote-rclone-goyle"


goyle is cute but may be more boring native or builtin thus to signal it's intimate relationship to rclone itself?

I have to say I don't understand what "goyle" or "garrgoyle" means at all! Do we need to expose it to users?

So I was trying to be clever with the name, but I've gotten pretty consistent feedback that the name is confusing!

We need to expose some name to users since they'll need to install a symlink somewhere (unless/until @joeyh adds a special-case to git-annex!).

I'll rebrand it as git-annex-remote-rclone-goyle and get rid of the "garrgoyle" name. Thanks, folks!

ncw

I think this is looking very good and ready to merge.

I'd like to make sure we get the naming right - that is the only thing that is difficult to change afterwards - your opinions on that would be great.

.gitignore

ncw · 2024-03-23T18:02:51Z

cmd/gitannex/gitannex.md

+
+2. Add a new external remote to your git-annex repo.
+
+   The new remote's type should be "rclone-goyle". If it is on your PATH,


Could the type just be rclone rather than rclone-goyle? As someone who knows very little about the protocols involved, the word goyle means nothing to me - is it important?

So if the name were just "rclone", then we would have a name collision with https://github.com/git-annex-remote-rclone/git-annex-remote-rclone. I'm working on changing the type to "rclone-builtin" right now, which seems reasonably self explanatory.

Back when I was thinking this would be a standalone command, rather than baked into rclone, I picked a globally-unique suffix that also made a semi-pun (git-annex-remote-rclone-goyle ==> "garrgoyle").

dmcardle · 2024-03-23T20:31:48Z

I think this is looking very good and ready to merge.

I'd like to make sure we get the naming right - that is the only thing that is difficult to change afterwards - your opinions on that would be great.

Hey @ncw, just got rid of the confusing name. Now it's git-annex-remote-rclone-builtin when invoked via symlink. I think this is ready to go!

Also, thank you to @joeyh and @yarikoptic for taking a look!

This commit adds a new subcommand named "gitannex", aka "git-annex-remote-rclone-builtin" when invoked via a symlink. This accomplishes milestone 1 from issue rclone#7625: "minimal support for the external special remote protocol". Issue rclone#7625

ncw

Thank you for this great feature :-)

I'll merge it now.

joeyh · 2024-03-26T17:49:41Z

@dmcardle good point that changes in git-annex would take quite a while to trickle down to eg debian stable. Of course, so will the new rclone.

I've opened a todo on git-annex https://git-annex.branchable.com/todo/external_special_remotes_not_using_git-annex-remote_in_name/

yarikoptic · 2024-03-26T19:13:48Z

FWIW a shameless plug which might come handy:

we provide non-official git-annex builds daily for the most of the OSes from https://github.com/datalad/git-annex/
https://github.com/datalad/datalad-installer also has rclone among its components with a good range of "methods" of installation: https://github.com/datalad/datalad-installer/?tab=readme-ov-file#rclone .

Overall -- might be easy to try the tandem as soon as we get rclone with it out.

dmcardle · 2024-03-26T19:28:57Z

Neat! Thank you for the context, @yarikoptic!

yarikoptic · 2024-03-26T20:47:24Z

cmd/gitannex/gitannex.md

+
+1. Create a symlink and ensure it's on your PATH. For example:
+
+        ln -s "$(realpath rclone)" "$HOME/bin/git-annex-remote-rclone-builtin"


I need to learn this dark magic (if it works ;-) yet to get a chance to try, but I have concerns ).

But ideally IMHO rclone itself should just install a shim itself in the PATH. The trickery that it would be different for *nix (shell script is enough) and Windows (needs .bat or .exe?)

Please do give it a try! I'm happy to make changes if there's something broken or risky.

The dark magic happens in gitannex.go's maybeTransformArgs(), which is called by init(). Basically, if argv[0] looks like "git-annex-remote-rclone-builtin", we insert the gitannex subcommand in the second position. There's actually already precedent in rclone for rewriting arguments like this: https://rclone.org/commands/rclone_mount/#rclone-as-unix-mount-helper.

It's decidedly a hack, but it means we don't have to write any platform-specific wrapper scripts. I think we can do the rough equivalent of ln -s on Windows with mklink, but I don't have a Windows machine handy to try it out.

I see a few paths forward that enable us to remove the symlink step from the setup instructions:

If git-annex adds a special case for this remote, i.e. @joeyh resolves this todo, users could just set the type or externaltype to whatever git-annex expects. No need for a symlink.

We could write wrapper scripts for every supported platform. Honestly, this is probably not that hard, but I'm generally wary of subtleties introduced by shell scripts.

We could write wrapper scripts for every supported platform and add a sub-subcommand (something like rclone gitannex installwrapper) that installs the platform-specific wrapper somewhere. That location could be alongside the rclone binary, but that makes me nervous. It could be a user-selected bin directory, but then the user still has to manage their PATH.

We could write wrapper scripts for every supported platform and ship them in rclone releases OR we could even ship relative symlinks in the releases, I suppose. This solves the package manager issue, but I'm not sure how project maintainers would feel about adding files to the release.

sorry -- my bad -- I was just too tired I guess. Instead of realpath rclone I saw rclone gitannex ;-)

Thanks for the analysis! yeah - we need @ncw's input on this. I guess such a wrapper could be done directly in go and just come with rclone package "natively" to not mess with "platform specific solutions" (no symlinks on windows really AFAIK).

I see a few paths forward that enable us to remove the symlink step from the setup instructions:

You could get rclone to create the symlink on the first writable element in PATH it finds...

I'd avoid scripting if possible - Go is marvelously consistent between platforms whereas shell/powershell isn't!

If we can build the logic into rclone then that makes it a one binary deploy which is what we aim for.

joeyh · 2024-04-17T19:30:29Z

@dmcardle I've implemented the rclone special remote in git-annex, so no symlink is needed and config looks like:

git-remote foo type=rclone encryption=none rcloneremotename=foo rcloneprefix=bar

Could you update the rclone documentation to reflect this?

dmcardle · 2024-04-18T13:22:37Z

Could you update the rclone documentation to reflect this?

Will do!

@joeyh, I have to admit I don't really understand the command you posted — is it supposed to be git annex initremote rather than git-remote?

FYI, I'm working on making this compatible with the various layout modes defined by git-annex-remote-rclone in #7754. Can you advise on how to switch an existing remote from externaltype=rclone to type=rclone? (Or is that a bad idea?) I'm hoping that users can transition to rclone's built-in remote without needing to copy their data.

dmcardle force-pushed the dmcardle-rclone-git-annex branch 3 times, most recently from 71bec9d to a4bc9db Compare March 4, 2024 02:09

dmcardle commented Mar 4, 2024

View reviewed changes

contrib/gitannex/git-annex-remote-rclone-goyle/main.go Outdated Show resolved Hide resolved

contrib/gitannex/git-annex-remote-rclone-goyle/main.go Outdated Show resolved Hide resolved

.gitignore Outdated Show resolved Hide resolved

dmcardle force-pushed the dmcardle-rclone-git-annex branch from a4bc9db to 27a0594 Compare March 4, 2024 17:48

dmcardle commented Mar 6, 2024

View reviewed changes

contrib/gitannex/gitannex.go Outdated Show resolved Hide resolved

contrib/gitannex/gitannex.go Outdated Show resolved Hide resolved

ncw reviewed Mar 7, 2024

View reviewed changes

contrib/gitannex/git-annex-remote-rclone-goyle/main.go Outdated Show resolved Hide resolved

contrib/gitannex/git-annex-remote-rclone-goyle/main.go Outdated Show resolved Hide resolved

contrib/gitannex/gitannex.go Outdated Show resolved Hide resolved

dmcardle force-pushed the dmcardle-rclone-git-annex branch 4 times, most recently from 66fc012 to 10b6b3d Compare March 10, 2024 01:49

dmcardle changed the title ~~Add contrib/gitannex/ package and "garrgoyle" commmand~~ cmd/gitannex: Add the gitannex command, aka "garrgoyle" Mar 10, 2024

dmcardle force-pushed the dmcardle-rclone-git-annex branch 2 times, most recently from 4071788 to 20ffa14 Compare March 11, 2024 21:47

yarikoptic reviewed Mar 22, 2024

View reviewed changes

ncw reviewed Mar 23, 2024

View reviewed changes

dmcardle force-pushed the dmcardle-rclone-git-annex branch from 20ffa14 to 0af0d16 Compare March 23, 2024 20:19

dmcardle changed the title ~~cmd/gitannex: Add the gitannex command, aka "garrgoyle"~~ cmd/gitannex: Add the gitannex command Mar 23, 2024

dmcardle force-pushed the dmcardle-rclone-git-annex branch from 0af0d16 to f738bb0 Compare March 23, 2024 20:29

dmcardle mentioned this pull request Mar 23, 2024

New subcommand that allows exports from git-annex? #7625

Open

dmcardle force-pushed the dmcardle-rclone-git-annex branch from f738bb0 to 067f8ce Compare March 23, 2024 20:57

dmcardle force-pushed the dmcardle-rclone-git-annex branch from 067f8ce to b0ba883 Compare March 25, 2024 21:16

ncw approved these changes Mar 26, 2024

View reviewed changes

ncw merged commit dfc329c into rclone:master Mar 26, 2024
10 checks passed

yarikoptic reviewed Mar 26, 2024

View reviewed changes

dmcardle mentioned this pull request Apr 1, 2024

cmd/gitannex: Replace e2e test script with Go command #7716

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/gitannex: Add the gitannex command #7654

cmd/gitannex: Add the gitannex command #7654

dmcardle commented Feb 29, 2024 •

edited

dmcardle commented Mar 4, 2024

ncw left a comment

dmcardle commented Mar 10, 2024

dmcardle commented Mar 11, 2024

dmcardle commented Mar 14, 2024

joeyh commented Mar 22, 2024

dmcardle commented Mar 22, 2024

yarikoptic Mar 22, 2024

dmcardle Mar 23, 2024

yarikoptic Mar 23, 2024

yarikoptic Mar 22, 2024

ncw Mar 23, 2024

dmcardle Mar 23, 2024

ncw left a comment

ncw Mar 23, 2024

dmcardle Mar 23, 2024

dmcardle commented Mar 23, 2024

ncw left a comment

joeyh commented Mar 26, 2024

yarikoptic commented Mar 26, 2024

dmcardle commented Mar 26, 2024

yarikoptic Mar 26, 2024

dmcardle Mar 26, 2024

yarikoptic Mar 27, 2024

ncw Mar 27, 2024

joeyh commented Apr 17, 2024

dmcardle commented Apr 18, 2024


		2. Add a new external remote to your git-annex repo.

		The new remote's type should be "rclone-goyle". If it is on your PATH,


		1. Create a symlink and ensure it's on your PATH. For example:

		ln -s "$(realpath rclone)" "$HOME/bin/git-annex-remote-rclone-builtin"

cmd/gitannex: Add the gitannex command #7654

cmd/gitannex: Add the gitannex command #7654

Conversation

dmcardle commented Feb 29, 2024 • edited

What is the purpose of this change?

Was the change discussed in an issue or in the forum before?

Checklist

dmcardle commented Mar 4, 2024

ncw left a comment

Choose a reason for hiding this comment

dmcardle commented Mar 10, 2024

dmcardle commented Mar 11, 2024

dmcardle commented Mar 14, 2024

joeyh commented Mar 22, 2024

dmcardle commented Mar 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ncw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmcardle commented Mar 23, 2024

ncw left a comment

Choose a reason for hiding this comment

joeyh commented Mar 26, 2024

yarikoptic commented Mar 26, 2024

dmcardle commented Mar 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeyh commented Apr 17, 2024

dmcardle commented Apr 18, 2024

dmcardle commented Feb 29, 2024 •

edited