-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
restorer: Add test for restore with include filter #1900
Conversation
e66aee9
to
b102959
Compare
traverseTree() was meant to call enterDir() whenever a directory is selected for restore, either explicitly or implicitly (=contains a file which is to be restored). After restoring a file, leaveDir() is called in reverse order for all intermediate directories so that the metadata can be restored. When a directory is selected implicitly, the metadata for it is restored. This is different from the previous restorer behavior, which created implicitly selected intermediate directories with permissions 0700 (only user can read/write it). This commit changes the behavior back to the old one. Only a directory is explicitly selected for restore, enterDir()/leaveDir() are called for it. Otherwise, only visitNode() is called, so visitNode() needs to make sure the parent directory exists. If the directory is explicitly included, leaveDir() will then restore the metadata correctly. When we decide to change the behavior (restore metadata for all intermediate directories, even if selected implicitly), we should do that in the selection functions, not here. This finally resolves #1870
Codecov Report
@@ Coverage Diff @@
## master #1900 +/- ##
==========================================
+ Coverage 51.06% 51.15% +0.09%
==========================================
Files 169 169
Lines 13097 13083 -14
==========================================
+ Hits 6688 6693 +5
+ Misses 5401 5383 -18
+ Partials 1008 1007 -1
Continue to review full report at Codecov.
|
Minor. I think there is no need to precreate
Update: I tried removing |
I think we should either isolate the (fairly complex) |
restorer: Add test for restore with include filter
What is the purpose of this change? What does it change?
Resolve an error with the restore command when an include filter is used.
Background:
traverseTree() was meant to call enterDir() whenever a directory is selected for restore, either explicitly or implicitly (=contains a file which is to be restored). After restoring a file, leaveDir() is called in reverse order for all intermediate directories so that the metadata can be restored.
When a directory is selected implicitly, the metadata for it is restored. This is different from the previous restorer behavior, which created implicitly selected intermediate directories with permissions 0700 (only user can read/write it).
This PR changes the behavior back to the old one. Only a directory is explicitly selected for restore, enterDir()/leaveDir() are called for it. Otherwise, only visitNode() is called, so visitNode() needs to make sure the parent directory exists. If the directory is explicitly included, leaveDir() will then restore the metadata correctly.
When we decide to change the behavior (restore metadata for all intermediate directories, even if selected implicitly), we should do that in the selection functions, not here.
This finally resolves #1870
Was the change discussed in an issue or in the forum before?
Closes #1870
Checklist
changelog/unreleased/
that describes the changes for our users (template here)gofmt
on the code in all commits