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 issues related to encoding #7791

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix issues related to encoding #7791

wants to merge 4 commits into from

Conversation

URenko
Copy link
Contributor

@URenko URenko commented Apr 22, 2024

Fix the way to disable encoding by introducing a new type Raw to solve #7456 and #6098.
See #7456 and the changes on the document for details.
The behavior of None was not changed to keep backward compatibility.

And fix vfs cache encoding described in #7760
The problem is when we create and write cache, we use the "OS" encoded ("OS" encoding is hardcoded for different operating systems, not specified by users).
But when we copy it to the remote (backend), we use local.Fs, which accept a name already encoded in "Standard" way.
So we need to send a "Standard" encoded name.

However, the "minimum setup" mentioned in #7760 has not been resolved. See my comment in it for details.
But it's an edge case.

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

@URenko URenko force-pushed the master branch 3 times, most recently from 1863b81 to 9d74a49 Compare April 22, 2024 14:16
@ncw
Copy link
Member

ncw commented Apr 22, 2024

The behavior of None was not changed to keep backward compatibility.

I think I'd prefer None to do what people expect and not do any encoding. What was your reasoning here?

@URenko
Copy link
Contributor Author

URenko commented Apr 23, 2024

The behavior of None was not changed to keep backward compatibility.

I think I'd prefer None to do what people expect and not do any encoding. What was your reasoning here?

Backward compatibility.
Since encoding has been introduced for a long time. There must be many people already misused None.
If we change the behavior and they upgrade it without reading the changelog, we will break their storage. For example, they will need to resync tons of files (this is what happened to me when encoding was introduced. I don't what it to happen again to others : ).
And If they read the changelog, they will change to Raw and reorganize things.

@URenko
Copy link
Contributor Author

URenko commented May 5, 2024

@ncw any progress or further concern? :-)

@URenko
Copy link
Contributor Author

URenko commented May 9, 2024

fixed #7824

Specifically, it causes statements like rclone copy . to spontaneously miss if . expands to a path with a Full Width replacement character.

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