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

Retain fs stats in artifacts zip #1609

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

rotu
Copy link

@rotu rotu commented Dec 17, 2023

The main driver of this is that artifact permissions are lost on upload:

actions/upload-artifact#37
actions/upload-artifact#38

But this will also keep file creation and modification timestamps to assist troubleshooting.

@rotu rotu requested a review from a team as a code owner December 17, 2023 09:26
@rotu rotu changed the title Include fs stats in zip archive Retain fs stats in zip archive Dec 17, 2023
@rotu rotu changed the title Retain fs stats in zip archive Retain fs stats in artifacts zip Dec 17, 2023
@rmunn
Copy link
Contributor

rmunn commented Apr 23, 2024

Looks like the mode parameter is also required in order to preserve Unix file permissions. The zip.append method that actions/toolkit uses is a tiny wrapper around the zip-stream package:

https://github.com/archiverjs/node-archiver/blob/0e0e9ceab65c35e8131701fd2b29ba3c28397385/lib/plugins/zip.js#L55

The entry method in zip-stream looks for the mode property on the incoming data:

https://github.com/archiverjs/node-zip-stream/blob/d2669793807f869da07b38e3e53a088e470702e2/index.js#L157

But nothing in zip-stream handles the stats property.

@rmunn
Copy link
Contributor

rmunn commented Apr 23, 2024

I've created #1723, which is essentially this PR plus passing the mode property into zip.append.

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.

2 participants