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
