-
Notifications
You must be signed in to change notification settings - Fork 5
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
[KDESKTOP-1262] Do not force file status after an upload session abortion #366
base: develop
Are you sure you want to change the base?
[KDESKTOP-1262] Do not force file status after an upload session abortion #366
Conversation
f22ce02
to
e26d2fc
Compare
const SyncPath &localPath = getFilePath(); | ||
static const std::vector<const char *> extendedAttributes = {EXT_ATTR_STATUS, EXT_ATTR_PIN_STATE}; | ||
|
||
for (const auto attribute: extendedAttributes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Questions.
- Should we remove all attributes or just
EXT_ATTR_STATUS
? (What is the definition of a placeholder for the application?) - Is a missing permission an actual problem for the user?
- In case of unexpected failure, should the exit code be changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove all attributes or just EXT_ATTR_STATUS? (What is the definition of a placeholder for the application?)
A file is a placeholder if it has the EXT_ATTR_STATUS
extended attribute (see LiteSyncExtConnector::vfsGetStatus
). So it should be enough to remove just this one. However, there is no point to keep only the pin state.
Moreover, the definition of a placeholder based solely on the status is also arguable. Perhaps basing it on the pinstate instead could help us display the correct status in Finder without risking to allow the deshydration of a not yet synced item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a missing permission an actual problem for the user?
Maybe. Perhaps we lose permissions during upload, which caused the abort. So the status has been set but cannot be removed, meaning that the file could be dehydrated while not synced, therefor corrupted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case of unexpected failure, should the exit code be changed?
Maybe again ^^ but what can we do if the access rights have been removed 🥲
…ad session abortion
740245b
to
87090d5
Compare
Quality Gate passedIssues Measures |
static const std::vector<const char *> extendedAttributes = {EXT_ATTR_STATUS, EXT_ATTR_PIN_STATE}; | ||
|
||
for (const auto attribute: extendedAttributes) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be in a utility function outside this class. Especially because normal
upload (meaning direct uploads, not upload session) are also concerned.
@@ -16,8 +16,8 @@ | |||
* along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
*/ | |||
|
|||
#import "../../../../src/libcommonserver/io/fileAttributes.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the common declarations in vfs.h ?
Or create a new dynamic library "libcommonserverext" with the common code.
// by the application when restarting the synchronization. | ||
|
||
const SyncPath &localPath = getFilePath(); | ||
static const std::vector<const char *> extendedAttributes = {EXT_ATTR_STATUS, EXT_ATTR_PIN_STATE}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the new function IoHelper::removeLiteSyncXAttrs
This pull request aims at addressing KDESKTOP-1262:
vfsForceStatus
withnullptr
after a drive upload session abortion.