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

Only try setting the execution permissions if the file doesn't have them already #110

Merged

Conversation

azubieta
Copy link
Contributor

@azubieta azubieta commented May 7, 2019

No description provided.

@azubieta
Copy link
Contributor Author

azubieta commented May 7, 2019

I'm also thinking on a way on creating a uni-test for this scenario. Any ideas @probonopd

@probonopd
Copy link
Member

Run the function on an AppImage that is already executable, and have the test fail if libappimage tries to set the executable bit again despite that it is already there?

@azubieta
Copy link
Contributor Author

azubieta commented May 9, 2019

fail if libappimage tries to set the executable

I have no way of intercept this without modifying the libappimage code.

@TheAssassin TheAssassin merged commit 0705fea into master May 9, 2019
@TheAssassin TheAssassin deleted the fix_set_execution_permissions_operation_not_permitted branch May 9, 2019 16:43
@probonopd
Copy link
Member

Why do you think the libappimage code should not be modified?

@azubieta
Copy link
Contributor Author

Why do you think the libappimage code should not be modified?

I guess that I didn't explain it properly. To the set execution permissions is performed by a single function call inside the register method. It would be required to extract that call and put it inside some kind of weird wrapper to be able to know from the tests environment whether it has been called or not. All of that will also reduce performance and will increase the complexity or our code base.

In this scenario the relation effort vs gain is really bad. That's why I don't want to do such modification on libappiimage.

The ideal use case will be having an AppImage file without execution permission that doesn't belongs to the current user. We cold create such file but once we do chown on it we will lose control and this may just lead to weird things.

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.

3 participants