newly created folders will be read-only when needed #6343
Conversation
|
not ready |
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
from
e767bbc
to
b724221
Compare
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
9 times, most recently
from
b5f7fdd
to
cd6413b
Compare
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
2 times, most recently
from
d5bc5cd
to
62d3388
Compare
|
windows build https://cloud.nextcloud.com/s/GPFmNcrTsoFqF4q |
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
2 times, most recently
from
43480d6
to
3a8167e
Compare
|
fyi, you need to update the signature in .drone.yml, otherwise drone will just wait for manual approval of the ci run, |
|
@mgallien drone compilation is broken, btw, unused parameter |
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
4 times, most recently
from
8185df1
to
040c00a
Compare
|
latest build for windows https://cloud.nextcloud.com/s/E9LtgzR8zkrPabL |
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
3 times, most recently
from
3cab70f
to
64e2b99
Compare
| } | ||
| catch (const std::filesystem::filesystem_error &e) | ||
| { | ||
| qCWarning(lcPropagateDownload) << "exception when checking parent folder access rights" << e.what() << e.path1().c_str() << e.path2().c_str(); |
There was a problem hiding this comment.
@mgallien What I really don't like is that we are using std::filesystem exceptions here on this level, while we never had them before in the sync engine. I would suggest handling them in the utility source files when possible. The code here now looks much more complicated if you just want to read the PropagateDownload job logic.
Besides, you don't have any logic in these catch blocks just a log.
There was a problem hiding this comment.
so you want to be forbid using std::filesystem APIs ?
constructors of std::filesystem::path can throw
what do you suggest ?
not using this type ?
There was a problem hiding this comment.
@mgallien No, using unified way of error processing in our API, and moving try/catch blocks of std::filesystem::path into our framework, like you id already in filesyste.cpp, then on the outside, from our API you can return an error message for example, or an optional that we already have for a result error is set or not, and if set, then there is an error message. But it is up to you. My motivation is to keep PropagateItem jobs more simple and easier to read. We have so much logic there.
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
2 times, most recently
from
d4387d6
to
0c0d306
Compare
| return false; | ||
| } | ||
|
|
||
| securityDescriptor.reset(new char[neededLength]); |
There was a problem hiding this comment.
@mgallien you could've used std::vector<char>::resize()
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
from
0c0d306
to
3c52a8b
Compare
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
should solve the AppImage build issue Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Close #6296 Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
Signed-off-by: Matthieu Gallien <matthieu.gallien@nextcloud.com>
mgallien
force-pushed
the
feature/applyPermissionsFromServerToFolders
branch
from
3c52a8b
to
ff9953b
Compare
|
AppImage file: nextcloud-PR-6343-ff9953b36b5644ec751682687f7257fe9cedfb2b-x86_64.AppImage |
|
|
/backport b0a2d5f to stable-3.12 |
Close #6296