Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

file put cmd: trailing slash on directories should be optional #259

Open
Thierry61 opened this issue Sep 29, 2019 · 8 comments
Open

file put cmd: trailing slash on directories should be optional #259

Thierry61 opened this issue Sep 29, 2019 · 8 comments

Comments

@Thierry61
Copy link

@Thierry61 Thierry61 commented Sep 29, 2019

One thing that is unnatural to me is that the result of safe file put command on a directory depends on the presence or absence of a trailing slash. Traditionally, results of commands don't depend on it, for example to move or copy a file to a directory the trailing slash on the directory is optional, for example cp f dir is the same as cp f dir/.

Command 1 (without a trailing slash):

$ ./safe files put -r ~/xtest
FilesContainer created at: "safe://hnyynywx759ws55xff97wfnrjn1r81w7frd3ku8m4ms6athnd1csxy76xrbnc"
+  /root/xtest/img/safe_logo_blue.svg  safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z
+  /root/xtest/index.html              safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk
$ ./safe cat safe://hnyynywx759ws55xff97wfnrjn1r81w7frd3ku8m4ms6athnd1csxy76xrbnc
Files of FilesContainer (version 0) at "safe://hnyynywx759ws55xff97wfnrjn1r81w7frd3ku8m4ms6athnd1csxy76xrbnc":
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| Name                          | Size | Created              | Modified             | Link                                                              |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/img/safe_logo_blue.svg | 5851 | 2019-09-29T19:08:56Z | 2019-09-29T19:08:56Z | safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/index.html             | 355  | 2019-09-29T19:08:56Z | 2019-09-29T19:08:56Z | safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+

Command 2 (with a trailing slash):

$ ./safe files put -r ~/xtest/
FilesContainer created at: "safe://hnyynyzmwa3arf58q3kqbny1gima4b4n7r1ymuecih5cch147t415nf3h4bnc"
+  /root/xtest/img/safe_logo_blue.svg  safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z
+  /root/xtest/index.html              safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk
$ ./safe cat safe://hnyynyzmwa3arf58q3kqbny1gima4b4n7r1ymuecih5cch147t415nf3h4bnc
Files of FilesContainer (version 0) at "safe://hnyynyzmwa3arf58q3kqbny1gima4b4n7r1ymuecih5cch147t415nf3h4bnc":
+-------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| Name                    | Size | Created              | Modified             | Link                                                              |
+-------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /img/safe_logo_blue.svg | 5851 | 2019-09-29T19:09:46Z | 2019-09-29T19:09:46Z | safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z |
+-------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /index.html             | 355  | 2019-09-29T19:09:46Z | 2019-09-29T19:09:46Z | safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk |
+-------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+

The destination path is /xtest for command 1 and / for command 2. The latter is the correct one because it is the default destination path when none is provided. To define a destination path other than / the user has to provide it explicitly.

Command 3 (with a destination path and a trailing slash on local path):

$ ./safe files put -r ~/xtest/ xtest
FilesContainer created at: "safe://hnyynyzigy73yzhzcisu36ne9qz4gzomzywj657wrgg8m3ggf7zpin4g5abnc"
+  /root/xtest/img/safe_logo_blue.svg  safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z
+  /root/xtest/index.html              safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk
$ ./safe cat safe://hnyynyzigy73yzhzcisu36ne9qz4gzomzywj657wrgg8m3ggf7zpin4g5abnc
Files of FilesContainer (version 0) at "safe://hnyynyzigy73yzhzcisu36ne9qz4gzomzywj657wrgg8m3ggf7zpin4g5abnc":
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| Name                          | Size | Created              | Modified             | Link                                                              |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/img/safe_logo_blue.svg | 5851 | 2019-09-29T20:01:15Z | 2019-09-29T20:01:15Z | safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/index.html             | 355  | 2019-09-29T20:01:15Z | 2019-09-29T20:01:15Z | safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+

This method is the correct (and existing) way to include a destination path. In this example it is the same as the directory, but it could be any other path. Here, the result doesn't depend on the presence of the trailing slash, which is a good thing:

Command 4 (with a destination path and no trailing slash on local path):

$ ./safe files put -r ~/xtest xtest
FilesContainer created at: "safe://hnyynyi6hrjqf7xanypp3tnjoobbpwrpppgaw5k1oqqmjt75zcxegmnr7wbnc"
+  /root/xtest/img/safe_logo_blue.svg  safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z
+  /root/xtest/index.html              safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk
$ ./safe cat safe://hnyynyi6hrjqf7xanypp3tnjoobbpwrpppgaw5k1oqqmjt75zcxegmnr7wbnc
Files of FilesContainer (version 0) at "safe://hnyynyi6hrjqf7xanypp3tnjoobbpwrpppgaw5k1oqqmjt75zcxegmnr7wbnc":
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| Name                          | Size | Created              | Modified             | Link                                                              |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/img/safe_logo_blue.svg | 5851 | 2019-09-29T20:08:38Z | 2019-09-29T20:08:38Z | safe://hbwynon5ey35si9umca83jkuo6t6qkmkwcp6s9t3g964d9u8fyippaza6z |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+
| /xtest/index.html             | 355  | 2019-09-29T20:08:38Z | 2019-09-29T20:08:38Z | safe://hbhybynoh189jbfes6u1qcu9ojff9rwap9dehr9neop6xikr5atdewactk |
+-------------------------------+------+----------------------+----------------------+-------------------------------------------------------------------+

In summary:

  • the trailing slash should be optional on directories
  • this is already implemented when a destination path is provided
  • this is not implemented when no destination path is provided, and it is here I find issue.
@Thierry61

This comment has been minimized.

Copy link
Author

@Thierry61 Thierry61 commented Oct 19, 2019

Note: Consistency with rsync command could be the reason to have implemented safe files put command that way.

Behavior of rsync command:

  • Without a trailing /: the last element of source path is root of destination path
  • With a trailing /: the last element of source path is omitted in destination path
$ rsync -r remote:xtest .
$ tree .
.
└── xtest
   ├── img
   │   └── safe_logo_blue.svg
   └── index.html

2 directories, 2 files
$ rm -rf xtest
$ rsync -r remote:xtest/ .
$ tree .
.
├── img
│   └── safe_logo_blue.svg
└── index.html

1 directory, 2 files

But as safe files put is a brand new command, this is to be balanced with the oddity of rsync behavior. Also going this way will open the door to demands for implementing all the peculiarities of this command.

@dan-da

This comment has been minimized.

Copy link
Contributor

@dan-da dan-da commented Nov 14, 2019

yeah I always found rsync "/" usage kinda weird.

@bochaco

This comment has been minimized.

Copy link
Member

@bochaco bochaco commented Dec 17, 2019

Very valid @Thierry61 and to me is still not clear where/how to find the balance for this thing.

When I was discussing this with other folks some time ago, I saw like some arguing against, but as many as those in favor (specially our IT folks back then found it natural), and it wasn’t clear enough to me it should be changed (and higher priorities came up also).

Personally I find it very easy and powerful that I can decide just with a slash if it’s a folder or its content/children that I’m uploading, and the same when I decide where I’m uploading it to. Although, I do understand that can also be easily missed and/or it could confuse others. It sounds to me this is probably related to different background of users which makes it look very strange and unnatural, and for others like a no brainer thing.

E.g. if the slash was ignored in the source how would you like to signal when you want to upload the folder and not only its children? i.e. "i want to upload folder ~/myfolder onto safe://mynrs/path1 so for example the file at ~/myfolder/file1.txt it’s then found at safe://mynrs/path1/myfolder/file1.txt".

Then, what if I want only the children of ~/myfolder to be uploaded onto the same destination path? if the slash is ignored from the source you then need to provide a destination path to make a distinction between these two use cases, a destination path which I presume you will also want an ending slash to be ignored from?, and if that destination path doesn’t exist entirely, should it be created? or it shouldn’t and it should fail? otherwise it sounds you’ll need an additional flag to tell the CLI what you expect to happen…?..

So what I see with the slashes in the source and destination you have all the flexibility and power for all the combinations of use cases in a very consistent way.

@dan-da

This comment has been minimized.

Copy link
Contributor

@dan-da dan-da commented Dec 17, 2019

If I'm not mistaken, we are basically discussing the / behavior of rsync vs cp.

Personally, I've always preferred cp behavior. That is probably because it is what I learned first and am most familiar with, so it seems "natural". I think that more people will be familiar/comfortable with cp behavior because it is used all the time on the local filesystem, whereas rsync is used more rarely.

Perhaps we should take a poll. :)

btw, your use cases, using cp behavior:

# copy ~/myfolder to safe://mynrs/path1
$ cp -r ~/myfolder safe://mynrs/path1

# copy ~/myfolder to safe://mynrs/path1, alternate method, same result.
$ cp -r ~/myfolder/ safe://mynrs/path1

# copy ~/myfolder to safe://mynrs/path1, alternate method, same result.
$ cp -r ~/myfolder/ safe://mynrs/path1/

#copy children of ~/myfolder to safe://mynrs/path1
$ cp -r ~/myfolder/* safe://mynrs/path1

As I recall, copy on windows works the same way as cp, so it would be more familiar to windows "power users" also.

@bochaco

This comment has been minimized.

Copy link
Member

@bochaco bochaco commented Dec 17, 2019

I think that's basically it as a summary @dan-da , with the exception that I think you wouldn't be able to copy a folder with a diff name in the destination? well...I think you need the -a flag for cp...never mind I just tried it and you can also rename it with cp

@dan-da

This comment has been minimized.

Copy link
Contributor

@dan-da dan-da commented Dec 18, 2019

yeah rename is fine.

$ cp -r ~/myfolder safe://mynrs/path1/myfolder.newname
@dan-da

This comment has been minimized.

Copy link
Contributor

@dan-da dan-da commented Dec 18, 2019

btw, I will just point out something a little bit subtle above. In this command:

$ cp -r ~/myfolder/* safe://mynrs/path1

The * is actually interpreted by the shell and individual filenames are passed to cp. This is called globbing or shell expansion. There might be issues or inefficiencies if the number of matching files is huge (thousands, millions). Also, exact globbing behavior/syntax may vary from one shell to another.

rsync may be using special / behavior to avoid the use of globbing for these or related reasons. (Though anyway, a user could avoid it in bash just by quoting the arg)

Related to this, and possibly something that should be opened as a separate issue, cp accepts any number of source arguments with the final argument interpreted as dest. This means one can do things like:

$ cp -r photo1.jpg photo2.jpg photo3.jpg photo4.jpg dest/

Which is what the * would expand to.

Also shell expansion offers an abbreviated way to filter files and send multiple args via ranges and sets:

$ cp -r myfolder/album[1-4] dest/
$ cp -r myfolder/album{5,6,7,8} dest/
$ cp -r myfolder/album[9-12]/photos{1,2}.jpg dest/

but afaict, safe files add/put only accept a single location argument right now, so would be incompatible with shell expansion. I think this would frustrate users a lot.

Examples from rsync man page:

               rsync -t *.c foo:src/

       ... Note that the expansion of wildcards on the commandline (*.c) into a list of
       files is handled by the shell before it runs rsync and not by rsync itself (exactly the 
       same as all other  posix-style  pro‐grams).
       The syntax for requesting multiple files from a remote host is done by specifying additional remote-host args  in  the  same
       style as the first, or with the hostname omitted.  For instance, all these work:

              rsync -av host:file1 :file2 host:file{3,4} /dest/
              rsync -av host::modname/file{1,2} host::modname/file3 /dest/
              rsync -av host::modname/file1 ::modname/file{3,4}
@wardbr

This comment has been minimized.

Copy link

@wardbr wardbr commented Dec 19, 2019

If the only/main advantage of the rsync way of doing things is to realize the same with typing less characters, I prefer the cp way. I remember struggling at first usage of the rsync cmd (a couple of years ago) with the '/' or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.