Lua version of the @sample

[bapt]

Summary

The bonus of this version being: sandboxed
Natively rootdir compliant.

Test Plan

exp-run : https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=249035

[antoine]

Are the permissions handled ? (owner/group/mode)
Previously they were handled by the cp -p

[bapt]

In D23617#518337, @antoine wrote:

Are the permissions handled ? (owner/group/mode)
Previously they were handled by the cp -p

Good catch, I haven't thought about that one. and the simple answer is no they are not.

[mat]

Mmmm, I'm sorry, but how is this better?
It add like 15 lines to the file, and now I absolutely cannot read what it does.

[manu]

Switch to a smaller version that uses the latest pkg lua functions addition,

[manu]

This is, of course, intended to be commited when we release 1.15 in ports-mgmt/pkg

[manu]

In D23617#518337, @antoine wrote:

Are the permissions handled ? (owner/group/mode)
Previously they were handled by the cp -p

persmissions are preserved now, pkg.filecopy always preserve the owner/group/mode

[manu]

In D23617#518945, @mat wrote:

Mmmm, I'm sorry, but how is this better?
It add like 15 lines to the file, and now I absolutely cannot read what it does.

Does it looks better now ?

[mat]

I wonder about the design choices behind having "magic" strings everywhere, those %1, %@ and all.

It would be much more readable to have like, an array, say, pkg.args[1] and pkg.args[2], so you could know the number of arguments with pkg.args.length or pkg.n_args or something.

In D23617#569709, @manu wrote:

This is, of course, intended to be commited when we release 1.15 in ports-mgmt/pkg

Well, no, this can only be committed when 1.15 is the oldest version that is supported.

Keywords/sample.ucl

25

There is so much magic going on here.

What is %@ ? What is %S+ ? I looked at pkg-lua-script(5) and it does not appear in it.

31–33

This is not what the script was doing before.

The script copies the .sample to the destination file if the destination file does not exist. There is absolutely no comparison involved. There should be a pkg.fileexists or something.

[manu]

In D23617#569765, @mat wrote:

I wonder about the design choices behind having "magic" strings everywhere, those %1, %@ and all.

It would be much more readable to have like, an array, say, pkg.args[1] and pkg.args[2], so you could know the number of arguments with pkg.args.length or pkg.n_args or something.

Those are expanded are package creation by the keywords framework.
There is not pkg lua object containing the args, it's replaced by the framework and the real value (passed to @sample) are put in the resulting lua script in the +MANIFEST file.

In D23617#569709, @manu wrote:

This is, of course, intended to be commited when we release 1.15 in ports-mgmt/pkg

Well, no, this can only be committed when 1.15 is the oldest version that is supported.

Do we support building ports from the head branch on a machine using quarterly packages ? That seems odd to me.
I don't mind waiting ~1 month between 1.15 is in ports-mgmt/pkg and this but waiting a full quarterly seems a bit much.

Keywords/sample.ucl

31–33

This is exactly what it does too.
Sure there is a "useless" cmp, I can see to add a fileexist (or use of the lua os. function maybe)

[bapt]

In D23617#569765, @mat wrote:

I wonder about the design choices behind having "magic" strings everywhere, those %1, %@ and all.

This is there since the begining of keywords, it is designed over the shell variables: $1 $2 $@ $# and do works the same, this is absolutely not anything which is related to lua at all. the usage of % for the variable name is based on the already existing since pkg_install (
expension variables: %F %B etc.

The expansion of %1 %2 %@ %# happens at the packaging time the the "variable" are not saved anywhere, so it is impossible to push them back as regular variables for the lua scripts (the same way we don't do it for shell).

[manu]

Keywords/sample.ucl

25

I copied the gsub from the old review but a simple test on %# will be enough. I'll update the code.

[manu]

Use pkg.stat instead of pkg.filecmp to check if the target file exists.
Use '%#' instead of gsub the '%@' variable.

[mat]

Looks ok. Could you indent the inside of the here documents one step so the only lines that begin with text are ucl?

[manu]

Ident lua script.

[mat]

I am accepting this as portmgr, I assume it works ;-)

[bapt]

We would make an exp-run just to be sure there is no elephant in the room

[mandree]

This is broken and causes leftover files in the poudriere testport security/ca_root_nss, see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=250415
Please revise.

[mandree]

There is also a bug on deinstall if a @sample keyword is used with only one argument that does NOT end in .sample: in that case, the configuration file is copied onto itself on install, and on deinstall, it is compared to itself, found matching, and removed. Arguably, this can also be caught in the stage-qa phase and would also be clobbered on update, but it should be flagged so that the offending package's pkg-plist can be fixed. I have seen and fixed such bugs in the wild.