bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.75k stars 3.99k forks source link

Problems with pkg_tar file permissions #2925

Closed mwoehlke-kitware closed 6 years ago

mwoehlke-kitware commented 7 years ago

Description of the problem / feature request / question:

pkg_tar is unable to preserve the mode of files being packaged and makes everything executable by default. The following should be fixed:

Environment info

damienmg commented 7 years ago

Why do you think that "Making everything executable by default is just wrong"? Seems like a bold statement.

The mode of the original file is a deterministic issue and does not means anything in the case of generated file, you can override it with modes starting next release.

Agree for the last 2 FR that would be nice to have.

mwoehlke-kitware commented 7 years ago

Why do you think that "Making everything executable by default is just wrong"?

Because execute is a permission. If Ye Olde UNIX didn't think that should be an exceptional case, there wouldn't be a permission for it.

Most files do not by default have execute permission (typically only the outputs of the C/C++ linker do). Normal text files are not supposed to have execute permission, nor is it in any way beneficial for them. (It's also obnoxious as it makes them green in ls output.)

Anyway, this is an easy fix; just default mode to None instead of "0555"... per the second AI, build_tar.py still doesn't preserve the original permissions (in fairness, this may be hard to do via Python), but unlike pkg_tar, it is already at least trying to DTRT by picking reasonable defaults based on whether or not the input files already have execute permission.

The mode of the original file is a deterministic issue and does not means anything in the case of generated file, you can override it with modes starting next release.

The mechanism for generating the files should already give a reasonable default, and if not, your build tree is already messed up (which is out of scope of this issue). The only files that should have execute permission are scripts and compiled binaries. In the latter case, the linker already does the right thing.

Not being able to magically determine what permissions the input files should have had is not a bug in pkg_tar :smile:.

damienmg commented 6 years ago

Jenkins: test this please.

This looks reasonable to me.

damienmg commented 6 years ago

oops commented on the issue instead of the PR

hlopko commented 6 years ago

As discussed in #3585 you can use mode" parameter in pkg_tar explicitly if needed.