Check and adjust permissions before kernel spec is written

[RobertRosca]

If the permissions for the resources directory do not allow the owner to write into it (e.g. as is the case in some directories using ACL, or if site-packages is set to read-only) then the install phase will fail with an error saying that kernel.json could not be written as the copied resources directory is read-only.

This PR fixes that by adding a permission check to write_kernel_spec. If resources has no write permissions it sets only the owner permission (leaving group/other unchanged) to 7 so that kernel.json can be written into it.

Also added in a test which creates a resources directory with read-only permissions to check that this has worked correctly. For the test to work write_kernel_spec now has a new resources optional argument, which is set to RESOURCES by default.

@takluyver :smile:

[takluyver]

I think the test can be a bit neater using the pytest tmp_path fixture, like this:

def test_write_kernel_spec_permissions(tmp_path):

Then pytest takes care of creating a temporary directory, gives you a pathlib Path object to work with, and cleans up afterwards.

[RobertRosca]

Oops, didn't see your comment, I changed it to use the fixture as you suggested.

I originally wrote it using the pathlib methods instead of os.path.join like this:

    read_only_resources = tmp_path.joinpath("_RESOURCES")
    shutil.copytree(RESOURCES, read_only_resources)

    # file used to check that the correct (mocked) resource directory was used
    with open(read_only_resources.joinpath('touch'), 'w') as f:
        pass

But, at least to me, the .joinpath thing seems a bit harder to read than os.path.join for something simple like this, so I'd rather leave it as it is.

[RobertRosca]

Thank you for pitching in, @RobertRosca, and sorry that this stalled. As I mention on a code inline comment - I believe the functional equivalent of the proposed change has already been merged in #593 (and unfortunately also overlooked earlier in #377).

Hey, no problem! That's alright, I understand these things can drop through the cracks (like you reply did for me :wink:) so it's no problem.

If you have the desire and time, I think it would make sense to isolate this PR to a test of that functionality, so we don't accidentally lose it in the future. We also probably want remove all those whitespace changes from the commit to keep things tidy and isolated to the new test case

Sure, that sounds good to me.

Let us know if you'd like to proceed or if we should press on by ourselves. Either way is fine, it's really up to you :)

I'm happy to carry on with this, but given it's been a while since you asked I want to check if this is all still needed.

Cheers!