[tools] Add chmod in tools.files #17800
Conversation
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
conan/tools/files/files.py
Outdated
| @@ -263,6 +264,47 @@ def chdir(conanfile, newdir): | |||
| os.chdir(old_path) | |||
|
|
|||
|
|
|||
| def chmod(conanfile, path:str, read:bool, write:bool, execute:bool, recursive:bool=False): | |||
There was a problem hiding this comment.
| def chmod(conanfile, path:str, read:bool, write:bool, execute:bool, recursive:bool=False): | |
| def chmod(conanfile, path:str, read:bool=None, write:bool=None, execute:bool=None, recursive:bool=False): |
Using None to mean "unchanged" for a chmod +x or chmod +w equivalent would provide a very useful and intuitive interface, e.g. chmod(self, some_exe, execute=True)?
conan/tools/files/files.py
Outdated
| :param execute: If ``True``, the file or directory will have execute permissions for any user. | ||
| :param recursive: (Optional, Defaulted to ``False``) If ``True``, the permissions will be applied | ||
| """ | ||
| if platform.system() == "Windows": |
There was a problem hiding this comment.
Why? ConanCenter recipes very rarely do this, only in a couple of occcasions.
And chmod in Windows still can change the read-only flag, so if the function has read argument, then it should work.
There was a problem hiding this comment.
Okay, but what should be done when passing execute or write? Just ignore or rise an error?
There was a problem hiding this comment.
Whatever the underlying os.chmod does. This is mostly a convenience wrapper for it, the overall expected behavior should be aligned.
conan/tools/files/files.py
Outdated
| def _change_permission(it_path:str): | ||
| mode = os.stat(it_path).st_mode | ||
| permissions = [ | ||
| (read, stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH), |
There was a problem hiding this comment.
I am not sure the default should be setting the permissions for everyone, but just for the current user.
Recipes in ConanCenter set S_IXUSR/S_IXEXEC only, not all.
There was a problem hiding this comment.
I know some recipes also do os.chmod(filename, os.stat(filename).st_mode | 0o111), but maybe this has to be checked with @jcar87 too
There was a problem hiding this comment.
Agreed. Mostly of the cases are related to tool-requires only.
There was a problem hiding this comment.
Ok, looking better now, still I wonder why the read permission will be different to the others, is it not enough that it has "user" read permissions (if for some reason it didn't have it)?
There was a problem hiding this comment.
Done, only changed owner permission.
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
Signed-off-by: Uilian Ries <[email protected]>
There was a problem hiding this comment.
Looking good to me.
Assigning it to @jcar87 for review and check regarding ConanCenter usage
conan/tools/files/files.py
Outdated
| def _change_permission(it_path:str): | ||
| mode = os.stat(it_path).st_mode | ||
| permissions = [ | ||
| (read, stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH), |
There was a problem hiding this comment.
Ok, looking better now, still I wonder why the read permission will be different to the others, is it not enough that it has "user" read permissions (if for some reason it didn't have it)?
Only leftover, gonna change it too. |
Signed-off-by: Uilian Ries <[email protected]>
Changelog: Feature: Integrate chmod feature in tools.files
Docs: TODO
The
tools.files.chmodis a wrapper foros.chmod, simplifying it's usage.It will change permissions only related to the file's owner and will follow
os.chmodbehavior as well. On Windows, chmod is limited to writing permission changes; adding executable permission is out of scope.Reference: https://docs.python.org/3/library/os.html#os.chmod
closes #8003
/cc @memsharded
developbranch, documenting this one.