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: check permissions before attempting chmod #363

Closed
wants to merge 2 commits into from
Closed

fix: check permissions before attempting chmod #363

wants to merge 2 commits into from

Conversation

RobCannon
Copy link
Contributor

Prerequisites

  • [x ] I have read and understand the CONTRIBUTING guide
  • [ x] The commit message follows the conventional commits guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Description

On a linux system, when you import the powershell module that was installed for AllUsers, the current user will usually not have the permissions to change the executable permission on the executable. That returns an error every time the module is imported.

This fix will check to see if the executable permission is already set and skip running chmod. It will also make sure the current user has the correct permissions to change the executable permissions before attempting to run chmod. If the module is installed by a sudo user, as long as the module is imported by that user, the file permissions will get set correctly.

@RobCannon RobCannon marked this pull request as ready for review January 21, 2021 00:26
Copy link
Owner

@JanDeDobbeleer JanDeDobbeleer left a comment

Choose a reason for hiding this comment

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

See my comments in-line 🙏🏻

Copy link
Contributor Author

@RobCannon RobCannon left a comment

Choose a reason for hiding this comment

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

Code has been moved to private function

$hasExecutable = $permissions[3] -eq 'x'
if ($hasWrite -and -not $hasExecutable) {
Invoke-Expression -Command "chmod g+x $executable"
}
Copy link
Owner

Choose a reason for hiding this comment

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

Return here as well, and the else can be removed.

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.

None yet

2 participants