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

Preserve mtime #84

Merged
merged 4 commits into from
Apr 13, 2022
Merged

Preserve mtime #84

merged 4 commits into from
Apr 13, 2022

Conversation

PVince81
Copy link
Member

@PVince81 PVince81 commented Apr 11, 2022

Now storing mtime as mtime metadata in the Zip file.

The import procedure has been adjusted to use the "stat" information
from the ZIP file which is accessible in sequential form, so the
recursion was removed. This assumed that the ZIP entries are always
correctly ordered.

For #83

@PVince81 PVince81 added the 3. to review Waiting for reviews label Apr 11, 2022
@PVince81 PVince81 requested review from Pytal and come-nc April 11, 2022 13:16
@PVince81 PVince81 self-assigned this Apr 11, 2022
@PVince81
Copy link
Member Author

one thing that is missing is that the folder mtime resets itself after writing into the folders, so we'd need to detect whenever we're done writing to a folder and then touch() said folder again

I'm not 100% sure about the current sequential approach.
The thing that bothered me about Zip::getFolder() is that it's not cached, so it will repeatedly re-traverse the whole list which can become slow for long lists

lib/ImportSource.php Outdated Show resolved Hide resolved
@come-nc
Copy link
Collaborator

come-nc commented Apr 11, 2022

Looks good I think.

@PVince81 PVince81 force-pushed the enh/83/preserve-mtime branch 2 times, most recently from 2c123e1 to 8529af5 Compare April 11, 2022 15:53
@PVince81 PVince81 mentioned this pull request Apr 12, 2022
@come-nc come-nc force-pushed the enh/83/preserve-mtime branch from 8529af5 to 0994f64 Compare April 12, 2022 14:07
@come-nc
Copy link
Collaborator

come-nc commented Apr 12, 2022

(rebased on main)

@PVince81 PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 12, 2022
Now storing mtime as mtime metadata in the Zip file.

The import procedure has been adjusted to use the "stat" information
from the ZIP file which is accessible in sequential form, so the
recursion was removed. This assumed that the ZIP entries are always
correctly ordered.

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81 PVince81 force-pushed the enh/83/preserve-mtime branch from 0994f64 to e65d249 Compare April 13, 2022 07:50
@PVince81
Copy link
Member Author

rebased again, hopefully now with updated API stubs

@PVince81
Copy link
Member Author

hmm, I only added getStat on ZIP but it says the method doesn't exist on Archive, maybe need to change the declared type

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
Adds an exception for getStat from OC\Archive\Zip which is in a private
namespace

Signed-off-by: Vincent Petry <vincent@nextcloud.com>
@PVince81
Copy link
Member Author

the "OC" private package is not in the included library with the stubs, so I've now updated the psalm baseline

@PVince81 PVince81 merged commit 78a1a5e into main Apr 13, 2022
@PVince81 PVince81 deleted the enh/83/preserve-mtime branch April 13, 2022 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants