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

Explicitly fix script permissions after copying #206

Merged
merged 4 commits into from Dec 17, 2020

Conversation

cottsay
Copy link
Member

@cottsay cottsay commented Dec 16, 2020

There are two reasons for this change:

  1. The permissions on these files may get changed if the source code is packaged into a format that doesn't support storing the permissions, and the current implementation relies on copying the files with the permissions intact.
  2. Some of these scripts are templates, which can't be executed prior to processing. Removing the executable permissions from the file makes this more clear.

This issue was discovered when RPM packaging automation attempted to detect the shell requirements for this package and tripped over the template syntax in some of the templates: https://bugzilla.redhat.com/1908156

There are two reasons for this change:
1. The permissions on these files may get changed if the source code is
   packaged into a format that doesn't support storing the permissions,
   and the current implementation relies on copying the files with the
   permissions intact.
2. Some of these scripts are templates, which can't be executed as-is.
   Removing the executable permissions from the file makes this more
   clear.
@cottsay cottsay added the bug Something isn't working label Dec 16, 2020
@cottsay cottsay self-assigned this Dec 16, 2020
cottsay and others added 2 commits December 16, 2020 11:21
Since the desired permissions are pretty obvious, it's easier to just
set the permissions instead of trying to add executable permissions to
the existing file.

Evidently os.chmod sets the permissions explicitly and isn't additive.
Copy link
Collaborator

@emersonknapp emersonknapp left a comment

Choose a reason for hiding this comment

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

LGTM

@emersonknapp emersonknapp merged commit abbb7ee into master Dec 17, 2020
@delete-merged-branch delete-merged-branch bot deleted the cottsay/template_perms branch December 17, 2020 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants