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

packaging: preserve chmod for the packaged files and folders #770

Closed
obastemur opened this issue Jan 8, 2016 · 4 comments
Closed

packaging: preserve chmod for the packaged files and folders #770

obastemur opened this issue Jan 8, 2016 · 4 comments

Comments

@obastemur
Copy link
Member

Currently, we do not hold file mode for neither files or folders. This is an issue especially when the package is created as an extract-able media.

@ktrzeciaknubisa
Copy link
Member

We do store mode for files (part of the stat object) but this is true, that not for folders. I have implemented this feature and I've got it ready, but I'd like to share few thoughts first:

  1. I guess this is feature makes sense only for extractable packages
  2. I propose, that we hold stat.mode for folders only as a single number - no need to embed entire stat object (this will help keeping minimal increase of jx package size)
  3. for the same reason, we don't need to store modes = 16877 (octal 755 - default for dir).
  4. also we don't need to set chmod for file modes = 33188 (octal 644 - default for file). This is for keeping extracting performance as good as possible.
  5. should this feature be enabled by default or only with dedicated option, e.g. --extract-chmod ?

@obastemur
Copy link
Member Author

for the same reason, we don't need to store modes = 16877 (octal 755 - default for dir).

Mind Android chmod since it's a bit different.

also we don't need to set chmod for file modes

Why ? What if the file is executable ?

This is for keeping extracting performance as good as possible.

Extracting to file system right ? If yes, how it makes much difference. We lost most time in IO file write.

should this feature be enabled by default or only with dedicated option, e.g. --extract-chmod ?

IMHO, default

@ktrzeciaknubisa
Copy link
Member

also we don't need to set chmod for file modes
Why ? What if the file is executable ?

The full sentence was:

also we don't need to set chmod for file modes = 33188 (octal 644 - default for file)

So I only meant to skip setting file mode if it's default 644. Other than that - we do set of course :)

@ktrzeciaknubisa
Copy link
Member

Feature implemented. After all I gave up on checking if chmod is default 644 (file) or 755 (dir) as it may vary between platforms.

So preserving chmod is enabled by default, but one may want to disable it and for this
--extract-chmod false can be used. It is also mentioned in docs.

Tested on OSX, android and Windows (just to check if nothing else got broke).

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

No branches or pull requests

2 participants