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

Rsync's behavior when uploading a directory #454

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

cdkharris
Copy link
Contributor

Rsync does not behave as described in this paragraph. I have set up an illustrating example for clarity.

These are my starting points on my local machine (a directory bar containing a file subbar) and the remote cluster.

Local:

$ ls -ld bar
drwxr-xr-x  3 ccaech0  staff  96 25 Jul 11:54 bar
$ ls -l bar 
total 0
-rw-r--r--  1 ccaech0  staff  0 25 Jul 11:54 subbar

Remote:

myriad $ ls -l bar subbar
ls: cannot access bar: No such file or directory
ls: cannot access subbar: No such file or directory

The example of L373 works as written.
Local:

$ rsync -avP bar myriad:~/
building file list ... 
2 files to consider
bar/
bar/subbar
           0 100%    0.00kB/s    0:00:00 (xfer#1, to-check=0/2)

sent 142 bytes  received 48 bytes  42.22 bytes/sec
total size is 0  speedup is 0.00

Remote:

myriad $ ls -l bar subbar
ls: cannot access subbar: No such file or directory
bar:
total 0
-rw-r--r-- 1 ccaech0 staff 0 Jul 25 11:54 subbar
myriad $ rm -rf bar subbar

Regarding the paragraph starting on L377, Rsync behaves identically if the trailing / on the target directory is omitted.

Local:

$ rsync -avP bar myriad:~ 
building file list ... 
2 files to consider
bar/
bar/subbar
           0 100%    0.00kB/s    0:00:00 (xfer#1, to-check=0/2)

sent 142 bytes  received 48 bytes  126.67 bytes/sec
total size is 0  speedup is 0.00

Remote:

$ ls -l bar subbar
ls: cannot access subbar: No such file or directory
bar:
total 0
-rw-r--r-- 1 ccaech0 staff 0 Jul 25 11:54 subbar
myriad $ rm -rf bar subbar

However, adding a trailing / on the source directory will result in the behaviour described.

Local:

$ rsync -avP bar/ myriad:~/
building file list ... 
2 files to consider
./
subbar
           0 100%    0.00kB/s    0:00:00 (xfer#1, to-check=0/2)

sent 138 bytes  received 48 bytes  74.40 bytes/sec
total size is 0  speedup is 0.00

Remote:

$ ls -l bar subbar
ls: cannot access bar: No such file or directory
-rw-r--r-- 1 ccaech0 staff 0 Jul 25 11:54 subbar

We have updated the text in the lesson to clarify this.

@reid-a
Copy link
Contributor

reid-a commented Jul 25, 2024

So both of your recent PRs reference a recent issue that's on a different repo. Propagating fixes across repos is awesome, please don't misunderstand, but it also looks like maybe you've issued a PR against the wrong repo? Can you clarify your intentions?

I'm going to look into it anyways, of course, a fix is a fix!

@reid-a
Copy link
Contributor

reid-a commented Jul 25, 2024

So having reviewed this, you're right that the text on our site is incorrect, for directory transfers, directory creation on the remote site occurs with or without the trailing slash on the destination, and this is true whether or not the target directory exists. The relevant trailing slash is on the source, as you correctly point out.

Interestingly, I also found this, which points out that a trailing slash on the destination can be important in the case where the source is a single file and the destination does not exist. rsync source path/to/dest creates a file at path/to/dest with the contents of source, but rsync source path/to/dest/ creates a directory path/to/dest with a file named source in it, at path/to/dest/source.

I like the second half of the PR. The comment about -r for scp is redundant with the immediately preceding discussion on L309 and L319.

I suggest you revise your PR to remove the changes around L344, but retain the others?

Copy link
Contributor

@reid-a reid-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to go, thanks!

@reid-a reid-a merged commit 61c0cb3 into carpentries-incubator:gh-pages Jul 25, 2024
10 checks passed
@cdkharris
Copy link
Contributor Author

So both of your recent PRs reference a recent issue that's on a different repo. Propagating fixes across repos is awesome, please don't misunderstand, but it also looks like maybe you've issued a PR against the wrong repo? Can you clarify your intentions?

I'm going to look into it anyways, of course, a fix is a fix!

Thanks! I'll explain what's happening:

At UCL we had not merged our site with the upstream carpentries-incubator/hpc-intro course for about 2 years, so the two have diverged. We are currently sorting through our local changes and grouping them into PRs. Most of these changes are updates to our site snippets, but there are some contributions to the lessons.

We made a plan for getting our site sufficiently up-to-date with the upstream so we can resume regular merges, which you've discovered in our issue that you've linked above. As part of that, I've been making PRs where it looked like our changes could be appropriate to contribute back. Happy to take a different approach :)

@cdkharris cdkharris deleted the rsync branch July 25, 2024 13:22
@reid-a
Copy link
Contributor

reid-a commented Jul 25, 2024

IMO this is a fine approach. We have had people do PRs against our repos from forks by mistake, so I wanted to be careful. Will dig into the other one soon!

@cdkharris
Copy link
Contributor Author

One of those mistake PRs was me ;)

Great, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants