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

drive: fix single Drive Document as FS root #2016

Merged
merged 1 commit into from Jan 26, 2018
Merged

Conversation

B4dM4n
Copy link
Collaborator

@B4dM4n B4dM4n commented Jan 26, 2018

Allow using Drive Documents as FS root by doing a direcoty list during NewFS.

Fixes #1772

Allow using Drive Documents as FS root by doing a direcoty list during NewFS.

Fixes rclone#1772
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.

Code looks great - will merge

See inline for ideas to fix a different ticket :-)

// return an error with an fs which points to the parent
return &newF, fs.ErrorIsFile
for _, e := range entries {
if _, isObject := e.(fs.Object); isObject && e.Remote() == remote {
Copy link
Member

Choose a reason for hiding this comment

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

I think this should probably check e.Remote() == remote first and do something if it was a directory too.

That will help with #1675

That issue is caused by a bug in drive which causes searching for directories with an accent not to be found so this branch of code will be entered when the thing missing is a directory too sometimes.

It isn't a 100% fix for #1675 but it would help.

Exactly what something should be I'm not sure, it it will probably involve poking the directory ID into directory cache...

I'm going to merge this as-is as it doesn't make #1675 any worse - do you fancy sending a follow-up for that issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I looked into the issue and think the workaround should not be handled in NewFS.
The workaround i propose for #1675 handles all directory lookup's and not just the root case.

Copy link
Member

Choose a reason for hiding this comment

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

OK that is fine with me :-)

@ncw ncw merged commit 29286cc into rclone:master Jan 26, 2018
@B4dM4n B4dM4n deleted the drive_fix_1772 branch January 27, 2018 11:21
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