hooks should be made executable instead of erroring out

[Michał Sawicz]

Related to bug #1792078 in some way.

snapcraft today complains if a hook isn't executable. It should just make it executable instead.

PR up here: https://github.com/snapcore/snapcraft/pull/2440

[Seth Arnold (seth-arnold)]

I dislike the idea of a tool blindly changing permissions on files.

Not only is it difficult to do safely, it might have unexpected consequences if metadata files or documentation or editor swap files etc are executed.

Thanks

[Michał Sawicz (saviq)]

@Seth it's only changing the permissions on the files as packaged, the files have no sense if they're not executable (and so snapcraft was erroring out in that case).

It's not touching the source tree or anything. It's also only changing the executable bit on files which are expected to be hooks. If for whatever reason they're files of any type you mention, I don't see how it would help, since the user being told they need to be executable, they'd just `chmod +x` it?

[John Lenton (chipaca)]

they need to either chmod it, or remove it. Depending on the case, the right thing to do might be one or the other. Why not remove it directly? Because that would be surprising to the developer.
Similarly, if the user didn't chmod it, why would you? The reason it errors out is because neither behaviour is the right one every time.

Case in point, somebody wanted to include a README in the hooks directory. How does making it executable accomplish anything?

[Michał Sawicz (saviq)]

I agree not all of the files should be made exec (I'll fix that in my PR), but for known hook names, to me it feels like an equivalent of

install -m 755 …

The problem is that when you check the code out onto a filesystem that doesn't support `chmod +x` (think NTFS partition), snapcraft will just error out and you can't do anything about it.

[Seth Arnold (seth-arnold)]

On Fri, Jan 18, 2019 at 05:47:16AM -0000, Michał Sawicz wrote:
> I agree not all of the files should be made exec (I'll fix that in my
> PR), but for known hook names, to me it feels like an equivalent of
>
> install -m 755 …

I like this approach a lot more. Thanks.

> The problem is that when you check the code out onto a filesystem that
> doesn't support `chmod +x` (think NTFS partition), snapcraft will just
> error out and you can't do anything about it.

Heh, this worry can lead to crazy things like text-mode FTP. :)

Thanks