maintainer-feedback requested: [Bug 253292] regression in r550860 (@sample conversion to lua) semantic change causes leftovers in poudriere, ex: security/ca_root_nss

[bugzilla-noreply at freebsd.org]

Matthias Andree <mandree at FreeBSD.org> has asked freebsd-pkg (Nobody)
<pkg at FreeBSD.org> for maintainer-feedback:
Bug 253292: regression in r550860 (@sample conversion to lua) semantic change
causes leftovers in poudriere, ex: security/ca_root_nss
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253292



--- Description ---
Greetings,

I am debugging a leftovers situation in security/ca_root_nss. Turns out that
since the Lua conversion, @sample (Keywords/sample.ucl) has different semantic
than the Shell version used to have.

I am looking at pkg 1.6.2, ca_root_nss 3.5.8, and ports from SVN at r564205
(that's the ^/head ummmm... trunk I'd say).

Here's how, when ca_root_nss's pre-deinstall script runs:

Situation:
1. pkg-plist contains:
@sample etc/ssl/cert.pem.sample

2. symlinks at deinstall time, after fresh installation of ca_root_nss package

$ ls -l /usr/local/etc/ssl/cert.pem.sample /usr/local/etc/ssl/cert.pem
lrwxr-xr-x  1 root  wheel  33 30 Jan. 02:23 /usr/local/etc/ssl/cert.pem.sample
-> ../../share/certs/ca-root-nss.crt
-rw-r--r--  1 root  wheel  786736 17 Okt. 18:23 /usr/local/etc/ssl/cert.pem

3. tracing with gdb into lua_pkg_filecmp() (you need to "set follow-fork-mode
child"):
(gdb) 
198		const char* file1 = luaL_checkstring(L, 1);
(gdb) 
199		const char* file2 = luaL_checkstring(L, 2);
(gdb) print file1
$1 = 0x80109b618 "/usr/local/etc/ssl/cert.pem.sample"
(gdb) print file2
$2 = 0x80109b758 "/usr/local/etc/ssl/cert.pem"
...
208		if (fstatat(pkg->rootfd, RELATIVE_PATH(file1), &s1,
AT_SYMLINK_NOFOLLOW) == -1) {
(gdb) 
212		if (fstatat(pkg->rootfd, RELATIVE_PATH(file2), &s2,
AT_SYMLINK_NOFOLLOW) == -1) {
(gdb) 
216		if (!S_ISREG(s1.st_mode) || !S_ISREG(s2.st_mode)) {
(gdb) 
217			lua_pushinteger(L, -1);

so it errors out here because file1 isn't regular.

HOW IS THIS DIFFERENT?
1) cmp -s in the earlier shell version of the script didn't care if it was
looking at regular files or symlinks or whatnot but would just open and compare
contents and exit 0 (same content)
2) the lua version now ERRORS out (-1) (and @sample ignores that and just
leaves the file) because it isn't looking at two regular files.

To me, it is not clear why pkg's lua_pkg_filecmp() cares so much about file
type WITHOUT following symlink. If it were, as a fallback, comparing symlinks,
that might have a selling point, but the way things are in pkg 1.6.2, it's not
clear to me.  Might rather be a quick sanity check (is the output something we
can mmap()) that misfires in corner cases. such as this.


OPTIONS:
1. change pkg's lua_pkg_filecmp() to follow symlinks or disregard unimportant
file type differences. Plus: keeps capsicum, fewer external commands.
2. revert the switch from shell to lua script made in ports r550860. Plus:
reinstates former behavior. Minus: loses capsicum isolation.
3. patch ALL ports that mix symlinks with @sample. Minus: doesn't scale.

I am proposing (1), i. e. bringing pkg closer to former cmp behavior.