Re: [nft PATCH 1/2] files: Drop shebangs from config files

[Phil Sutter <phil@nwl.cc>]

Hi Arturo,

On Tue, Nov 12, 2019 at 12:15:07PM +0100, Arturo Borrero Gonzalez wrote:
> On 11/7/19 12:45 PM, Phil Sutter wrote:
> > These are not meant to be executed as is but instead loaded via
> > 'nft -f' - all-in-one.nft even points this out in header comment.
> > While being at it, drop two spelling mistakes found along the way.
> > 
> > Consequently remove executable bits - being registered in automake as
> > dist_pkgsysconf_DATA, they're changed to 644 upon installation anyway.
> > 
> > Also there is obviously no need for replacement of nft binary path
> > anymore, drop that bit from Makefile.am.
> 
> If you drop the shebang, the shell may not know how to execute these files. Why
> not executing them with the python interpreter instead of `nft -f`?

Even without dropping it, shell won't execute them because we don't
install them with executable bit set.

> As pablo commented, the intention was to allow simple use cases like:
> 
> root@server:~# ./load-my-ruleset.nft
> 
> This use case would still be allowed after this patch but it would be a little
> less obvious (less examples). So I'm not sure about ACK'ing this patch.

While it is inconvenient for users to set the file executable first,
adding a shebang is certainly beyond that. IMO, we basically have two
options:

A) Apply my patch and stick to all-in-one.nft's header comment ("This
   script is meant to be loaded with `nft -f <file>`").

B) Ignore my patch and declare the configs as dist_pkgsysconf_SCRIPTS
   (untested) so they are installed with executable bit set.

Personally I find it awkward to directly execute files in /etc other
than sysv init scripts, hence why I prefer (A). For an example of "real"
nft scripts, there are the samples in files/examples/ which get
installed into $docdir/examples/ with executable bit set if my other
patch is applied.

But for me, (B) is fine as well. I just think we should be consistent.
:)

Cheers, Phil