search

conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.25k stars 980 forks source link

[feature] Interlize chmod as Conan tool #8003

Open uilianries opened 4 years ago

uilianries commented 4 years ago

Hi!

As you may know, chmod is a nice Unix tool, which is used to handle file permission.

On Python, we have os.chmod, but it's limited in terms of solution on Windows (see #7966).

Doing a quick search on CCI we have os.chmod present there, but there is no pattern for its usage, sometimes we force the permission number, sometimes we just update (add/remove) what we need.

My suggestion is adding a wrapper for chmod:

tools.chmod(os.path.join(self.package_folder, "bin", "foo"), stat.S_IEXEC)

Or

tools.chmod_x(os.path.join(self.package_folder, "bin", "foo"))
tools.chmod_w(os.path.join(self.package_folder, "bin", "foo"))
tools.chmod_r(os.path.join(self.package_folder, "bin", "foo"))

Or

tools.chmod(os.path.join(self.package_folder, "bin", "foo"), read=True, write=False, exec=True)

A plus for the third option: Windows users/developers may not know about numerical permission.

Also, as os.chmod is limited for Windows, we could include the project oschmod, which is able to fix the permission on Windows. Thus, tools.chmod would be effective for any OS. There is a nice blog post about oschmod on Medium.

/cc @a4z @solvingj

solvingj commented 4 years ago

I've wanted this kind of tool for my conan recipes in the past. I don't know enough about the various scenarios which require all the parameters of os.chmod to know if there's a "general case" for Conan to add a tool with sensible defaults. I hope so. There probably is, since we collapsed most of subprocess down to self.run, but I imagine some people will find reasons to disagree that this is in scope for Conan. Will be interesting to see.

memsharded commented 4 years ago

https://github.com/YakDriver/oschmod has 3 stars in Github, I wouldn't rely on this, version 0.3.12 in PyPI. I wouldn't rely on this yet. Plus I would like to know real cases in ConanCenter that requires this level of permission modification for Windows.

This are the current occurrences in ConanCenter

recipes\7zip\19.00\conanfile.py:
  65              fn = os.path.join("CPP", "Build.mak")
  66:             os.chmod(fn, 0o644)
  67              tools.replace_in_file(fn, "-MT", "-{}".format(str(self.settings.compiler.runtime)))

recipes\cunit\all\conanfile.py:
  63              for f in glob.glob("*.c"):
  64:                 os.chmod(f, 0o644)
  65  

recipes\depot_tools\all\conanfile.py:
  50          def chmod_plus_x(name):
  51:             os.chmod(name, os.stat(name).st_mode | 0o111)
  52  

recipes\freetype\all\conanfile.py:
  133          if os.name == "posix":
  134:             os.chmod(filename, os.stat(filename).st_mode | 0o111)
  135  

recipes\gmp\all\conanfile.py:
  53                  configure_stats = os.stat(configure_file)
  54:                 os.chmod(configure_file, configure_stats.st_mode | stat.S_IEXEC)
  55              configure_args = []

recipes\libdb\all\conanfile.py:
  186                          binpath = os.path.join(bindir, fn)
  187:                         os.chmod(binpath, 0o755)  # Fixes PermissionError(errno.EACCES) on mingw
  188                          os.remove(binpath)

recipes\meson\all\conanfile.py:
  42          if os.name == 'posix':
  43:             os.chmod(filename, os.stat(filename).st_mode | 0o111)
  44  

recipes\ninja\1.9.x\conanfile.py:
  55              name = os.path.join(self.package_folder, "bin", "ninja")
  56:             os.chmod(name, os.stat(name).st_mode | 0o111)
  57          self.env_info.PATH.append(os.path.join(self.package_folder, "bin"))

recipes\nspr\all\conanfile.py:
  146              for f in os.listdir(os.path.join(self.package_folder, "lib")):
  147:                 os.chmod(os.path.join(self.package_folder, "lib", f), 0o644)
  148  

recipes\openssl\1.x.x\conanfile.py:
  705              with tools.chdir(os.path.join(self.package_folder, "lib")):
  706:                 os.chmod("libssl.so.1.0.0", 0o755)
  707:                 os.chmod("libcrypto.so.1.0.0", 0o755)
  708  

recipes\waf\all\conanfile.py:
  45  
  46:         os.chmod(os.path.join(binpath, "waf"), 0o755)
  47:         os.chmod(os.path.join(binpath, "waf-light"), 0o755)
  48  

recipes\zlib\1.2.11\conanfile.py:
  37              st = os.stat(configure_file)
  38:             os.chmod(configure_file, st.st_mode | stat.S_IEXEC)
  39  

recipes\zlib\1.2.8\conanfile.py:
  45              st = os.stat(configure_file)
  46:             os.chmod(configure_file, st.st_mode | stat.S_IEXEC)
  47

It seems a wrapper that would simplify a bit the UI and avoid the st.st_mode | stat.S_IEXEC could be nice.

uilianries commented 4 years ago

YakDriver/oschmod has 3 stars in Github, I wouldn't rely on this, version 0.3.12 in PyPI. I wouldn't rely on this yet.

We could internalize its code.

Plus I would like to know real cases in ConanCenter that requires this level of permission modification for Windows.

The chmod has no effect on windows, we would need subprocess as @solvingj commented, but it would be nice if we could use API call instead, safer and faster than a fork. I can see few cases, most when we need to add exec permission for scripts (.sh, .py, ...) on Windows. I think any .exe file will be executable by default on Windows.

memsharded commented 4 years ago

The chmod has no effect on windows, we would need subprocess as @solvingj commented, but it would be nice if we could use API call instead, safer and faster than a fork. I can see few cases, most when we need to add exec permission for scripts (.sh, .py, ...) on Windows. I think any .exe file will be executable by default on Windows.

I mean, what is the real need. What library or Conan package requires this level of changing permissions. What cannot be built in ConanCenter unless using this code. It doesn't matter much if it doesn't have effect on Windows as long as libraries link and executables run.

uilianries commented 4 years ago

I mean, what is the real need. What library or Conan package requires this level of changing permissions. What cannot be built in ConanCenter unless using this code. It doesn't matter much if it doesn't have effect on Windows as long as libraries link and executables run.

Maybe not so focused on ConanCenter, but maybe for companies which are packaging internal packages.

We have a thread on Conan Slack channel: https://cpplang.slack.com/archives/C41CWV9HA/p1604505594005800?thread_ts=1604505490.005700&cid=C41CWV9HA /cc @a4z

memsharded commented 4 years ago

Maybe not so focused on ConanCenter, but maybe for companies which are packaging internal packages.

It is not clear from that thread what is the companies use case either. I think it is fine to add a Conan tool to abstract away the details of os.chmod a bit, but so far I don't see enough evidence to add or to depend on YakDriver/oschmod into our codebase.

a4z commented 4 years ago

as a package creator, when needing to be sure an executable has execution permission, it would be nice to have something like tools.make_executable(filename) and not be forced to add repetitive boiler plate, and maybe .

This will improve the readability dramatically, since today various names and solutions for the same thing are used but it will also improve the writing and authoring experience of packages, and welcome new recipe authors and maintainers.

- About implementation details, I do not have any strong opinion on what the best way is, but it would deffinitely be nice to get rid of the if not windows then chmod (and hopefully get all the details right) and on windows no idea

a4z commented 4 years ago

PS: my current use case https://github.com/cross-language-cpp/djinni-generator

the Windows .bat file is exactly the same file as the non .bat file, some kind of dark magic that works on all platforms (Linux OSX Windows) as long as there is some java findable (btw, can we please proceed with https://github.com/conan-io/conan-center-index/pull/2756 ;-) If it is somehow possible to not be in need of having a separate name for Windows, and the normal djinni file could be called this would be even more magic.

But please note, if you find that this use case is strange or too special
the main reason for my request for a tool is still what I wrote above https://github.com/conan-io/conan/issues/8003#issuecomment-721934163