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

Fix uploadfile only works for root of the fs #4490

Merged
merged 2 commits into from Aug 10, 2020

Conversation

negative0
Copy link
Member

What is the purpose of this change?

This bug ignored the remote parameter while uploading a file, thus the file will appear only in the root of the fs. This PR fixes that and adds test for it

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

No

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 :-)

Copy link
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

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

Thank you for fixing this!

See inline comments.

@@ -253,7 +254,7 @@ func rcSingleCommand(ctx context.Context, in rc.Params, name string, noRemote bo
return nil, err
}
if p.FileName() != "" {
obj, err := Rcat(ctx, f, p.FileName(), p, time.Now())
obj, err := Rcat(ctx, f, filepath.Join(remote, p.FileName()), p, time.Now())
Copy link
Member

Choose a reason for hiding this comment

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

That should be path.Join not filepath.Join. filepath is for OS native strings so will have \ instead of / on Windows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ahh I see. Will update that

@@ -450,11 +451,13 @@ func TestUploadFile(t *testing.T) {
testFileName := "test.txt"
testFileContent := "Hello World"
r.WriteFile(testFileName, testFileContent, t1)
testItem1 := fstest.NewItem(testFileName, testFileContent, t1)
testItem2 := fstest.NewItem(filepath.Join("subdir", testFileName), testFileContent, t1)
Copy link
Member

Choose a reason for hiding this comment

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

s/filepath/path/ here too

@negative0
Copy link
Member Author

Should this be failing?

Copy link
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

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

That looks great now - thank you

Don't worry about the failed test - it is irrelevant I think.

I'll merge it now!

@ncw ncw merged commit 61c7ea4 into rclone:master Aug 10, 2020
@negative0
Copy link
Member Author

Thanks!

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.

None yet

2 participants