The bonus of this version being: sandboxed
Natively rootdir compliant.
Details
Details
- Reviewers
manu - Group Reviewers
portmgr O5: Ports Framework - Commits
- rP550860: Lua version of the @sample
Diff Detail
Diff Detail
- Repository
- rP FreeBSD ports repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Are the permissions handled ? (owner/group/mode)
Previously they were handled by the cp -p
Comment Actions
Good catch, I haven't thought about that one. and the simple answer is no they are not.
Comment Actions
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.
Comment Actions
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.
Well, no, this can only be committed when 1.15 is the oldest version that is supported.
| Keywords/sample.ucl | ||
|---|---|---|
| 25 ↗ | (On Diff #74687) | 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 ↗ | (On Diff #74687) | 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. |
Comment Actions
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.
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 ↗ | (On Diff #74687) | This is exactly what it does too. |
Comment Actions
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).
| Keywords/sample.ucl | ||
|---|---|---|
| 25 ↗ | (On Diff #74687) | I copied the gsub from the old review but a simple test on %# will be enough. I'll update the code. |
Comment Actions
Use pkg.stat instead of pkg.filecmp to check if the target file exists.
Use '%#' instead of gsub the '%@' variable.
Comment Actions
Looks ok. Could you indent the inside of the here documents one step so the only lines that begin with text are ucl?
Comment Actions
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.
Comment Actions
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.