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

fix: permissions of shell completion files #24

Merged
merged 1 commit into from
May 17, 2024
Merged

Conversation

bchmnn
Copy link
Contributor

@bchmnn bchmnn commented May 17, 2024

This PR adds os.chown calls for the shell completion files. This is due to isisdl may be installed in a read-only way (e.g. on NixOS).

This may lead to problems that:

  1. shutil.copy preserves the ownership flags of the files that are being copied
  2. the source shell completion files are read-only; therefore, the shell completion files in .local/share/... are read-only
  3. on the second run isisdl tries to override shell completions files in .local/share that are read-only; isisdl fails.

This PR simply updates the ownership flags of the copied files in .local/share/... to be RW for the owner.

PS: Once this is hopefully merged and released, I'll add isisdl to nixpkgs (NixOS packet manager). :)

@Emily3403
Copy link
Owner

Great, thanks a lot! Thank you for fixing this 😊

@Emily3403 Emily3403 merged commit 39b2251 into Emily3403:main May 17, 2024
1 of 7 checks passed
@bchmnn
Copy link
Contributor Author

bchmnn commented May 17, 2024

@Emily3403 apparently the ci/cd failed? You sure it is ready for merging?

@Emily3403
Copy link
Owner

Yes, because secrets are not transferred when running workflows from a PR. I don't really have a clue how to fix this, so I checked it out locally and everything worked fine. The resulting action completed successfully.

@bchmnn
Copy link
Contributor Author

bchmnn commented May 17, 2024

Ah okay. Makes sense :)
Awesome. You can ping me, once the new release is on PyPi. Then I'll package isisdl for NixOS.

@Emily3403
Copy link
Owner

Should be released now

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