Skip to content

Conversation

@terofeev
Copy link
Contributor

Type of change(s)

  • Bug fix
  • Feature / enhancement
  • Infrastructure / tooling (CI, build, deps, tests)
  • Documentation

What changed and why

  • Preserve the original lastModifiedTime of files when unzipping them.

Fixes the following issue(s)

Checklist

  • I read the contribution guidelines.
  • I updated the "Unreleased" section in CHANGELOG.md (if applicable).
  • I manually tested my changes on device/emulator (if applicable).

@naveensingh naveensingh self-assigned this May 21, 2025
@naveensingh
Copy link
Member

I knew someone would take this on. I found a couple of issues:

  • It won't work when decompressing files directly using the action menu. See ItemsAdapter.kt.
  • Not explicitly included in the bug report, but it makes sense to fix this for compressions as well. Passing on the source timestamp when configuring zip parameters should be enough.
  • We should handle the case where the library returns 0 as the timestamp. We can use System.currentTimeMillis() or fallback to the previous no-op behavior.
  • Optionally, maybe consider doing this for directories as well? It'll likely complicate the restoration logic due to the sequential nature of extraction, but worth a try.

Thanks.

@naveensingh naveensingh added the waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. label May 21, 2025
@terofeev terofeev force-pushed the unzip-preserve-time branch from ec02dba to 0e267f1 Compare May 21, 2025 13:59
@terofeev
Copy link
Contributor Author

I knew someone would take this on. I found a couple of issues:

  • It won't work when decompressing files directly using the action menu. See ItemsAdapter.kt.
  • Not explicitly included in the bug report, but it makes sense to fix this for compressions as well. Passing on the source timestamp when configuring zip parameters should be enough.
  • We should handle the case where the library returns 0 as the timestamp. We can use System.currentTimeMillis() or fallback to the previous no-op behavior.
  • Optionally, maybe consider doing this for directories as well? It'll likely complicate the restoration logic due to the sequential nature of extraction, but worth a try.

Thanks.

Thanks for the great points, @naveensingh

I think it makes sense to create a separate issue for the compression side, since it’s a bit outside the scope of the original bug. The other three decompression-related points can definitely be handled in this PR.

Does that sound okay to you?

@naveensingh
Copy link
Member

Sure, let's go with that.

@terofeev terofeev force-pushed the unzip-preserve-time branch from 0e267f1 to a4f8460 Compare May 22, 2025 11:35
- Additionally handle the case with 0 last modified time.
@terofeev
Copy link
Contributor Author

Fixed the following cases:

  • Decompressing files via the action menu in ItemsAdapter.kt
  • Handling cases where the library returns 0 as the timestamp

I also attempted to update the "last modified" time for directories, but it's more complex than for regular files:

  • If a file is located in multiple nested folders, we need to update timestamps recursively (the current logic skips nested folders)
  • The logic in DecompressActivity skips empty folders entirely

So, I believe this PR addresses the primary issue. I’ll create two additional issues that are out of scope for this PR:

  • Passing the source timestamp when compressing files
  • Fixing timestamps when decompressing folders

What do you think? @naveensingh

@naveensingh naveensingh removed the waiting for author If the author does not respond, the issue will be closed. Otherwise, the label will be removed. label May 25, 2025
Copy link
Member

@naveensingh naveensingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good to me. But if directories are getting too complicated, we can skip them for now and just focus on files.

Merging since this works now 🎉

Thank you.

@naveensingh naveensingh merged commit ef23996 into FossifyOrg:master May 25, 2025
4 of 5 checks passed
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.

Preserve timestamps when extracting (.zip) archives

2 participants