Make chmod hermetic

[mattyclarkson]

Removes the need for system binaries to make the Python interpreter library directories/files read-only.

Fixes #2016

[aignas]

I think we should use the python_bin var that is defined below instead of using the host_python. And we should do the read-only fix only if the host platform the same as the platform of the toolchain.

Could we recursively change this for windows as well? Right now we are calling this only if it is not Windows, but do the scripts work on Windows?

[mattyclarkson]

os.chmod says:

Although Windows supports chmod(), you can only set the file’s read-only flag with it (via the stat.S_IWRITE and stat.S_IREAD constants or a corresponding integer value). All other bits are ignored.

So, yes, it would remove write access on Windows.

os.getuid is not available on Windows but I can find a fallback for that.

[mattyclarkson]

The chmod.py does set the folders and files to read-only on Windows, but does not prevent new files being created in the directories. So the same behaviour as Unix is not present. We would have to look into Windows permissions to restrict that.

The getuid could return the Window Security ID for the user but that doesn't really help much. I've removed the touch check by checking platform.

We now run the scripts on Windows, but they don't really do much and actually fail to prevent .pyc files being written. Which is no different to before this patch.

[mattyclarkson]

One can prevent writing files into directories on Windows with security permissions. That could be done by calling out to icacls.exe (in leiu of the win32security dependency). That will likely take significant extra testing as I'm not a Windows expert.

[mattyclarkson]

@aignas would you like me to continue figuring out truely read-only directories on Windows or does this PR move the needle enough to be considered for merge? I'd be happy to raise follow up issues/PRs, if necessary.

[aignas]

@rickeylev, what do you think about this? How do we move with this forward? Should we just start ignoring the pyc files in the repo and not bother with making it read-only?. The making of the external repos read-only is something that we don't do in the whl_library (maybe we should) and I am wondering what is the overall strategy here...

I am thinking that having the chmod, id and touch be hermetic might be ignoring the overall picture of what we are trying to do here.

[rickeylev]

what is the overall strategy here

So, correctness wise, there are two things we're constrained by:

1. The input files (both content and the list of them) have to be stable.
2. The downloaded python binary may not be runnable by the host that downloaded it, such as in a cross-build with RBE situation.

(1) can be accomplished by any of:

• a. Making everything read only.
• b. Generating all the necessary files at repo install time.
• c. Excluding files that might be created at runtime

I think we should do (a) regardless -- its just defense in depth. For the case of Windows (or other failures) we should just be OK with leaving things writable. It's the best we can do, and users will just bypass it entirely with ignore_root_error=True anyways (they are given the choice between "guaranteed pedantic failure" and "probably works, but flaky"; they'll always choose the latter).

I think (b) is better than (c), but it's out of scope for this PR and we already have (c) in place.

(2) throws a wrench into what this PR is doing -- using the downloaded runtime itself to run commands to make things read only. So I'm not really keen on that overall. I don't mind having an alternative implementation of chmod for OP's case where coreutils aren't available. Ideally, we'd either have a repo-phase reference to a host-compatible Python, or a repo-phase reference to a host-compatible chmod tool.

So, more concretely, I ask for the following changes:

• Use the system chmod if available, otherwise use the downloaded-python's chmod. If neither is available, oh well; continue on.
• Remove the fail() code paths. Just chmod and trust that things are OK.

[mattyclarkson]

I've updated to use uutils/coreutils and removed the fail paths.

[aignas]

Speculation, that needs to be measured: The way this is implemented it would fetch and extract the chmod binary for each python version. It would use the bazel repository cache, but having the chmod in a separate repository might be better from the performance point of view?

In general, using the chmod that is hermetic is an interesting idea, but I wonder if we should use the system chmod if it exists and if it doesn't, download the hermetic chmod. That would also satisfy the ask to support your usecase where system chmod is not available. All of this could be done in a separate host_chmod repository, we could even switch the behaviour to default to the hermetic chmod if an env var is set to a particular value.