• Actually remove the directory (add -r)
• Do not overwrite previous traps (otherwise the temporary /cfg directory is not removed).

Address suggestions:

• Do not wrap strings

In D48778#1112946, @emaste wrote:

We could add a check for double slash to tools/pkgbase/metalog_reader.lua also

Address suggestions:

In D47722#1090657, @jlduran wrote:

Also, I have yet to investigate if the mtree parsing under makefs is too strict (https://cgit.freebsd.org/src/tree/usr.sbin/makefs/mtree.c#n1004). But I'll check that once I have finished polishing building NanoBSD without root privileges (https://reviews.freebsd.org/D47653#1086475, https://www.youtube.com/watch?v=5qCaOMQ3ZnQ&t=2917s).

After a simple:

grep -r -A3 "case 10" contrib/bsddialog/

I could not find any other occurrence that needs fixing.

If it helps, the diffs for blocklistd are OK.
I keep a local diff for just blocklistd: (https://github.com/jlduran/openssh-portable), just in case we decide to upstream our diffs.

• Maintining the previous fix, but with a simpler approach

Thank you for taking a look at this!
Not only it fixes D47534, it also does not break D47585 (introduce a regression).

Address fixes:

Address suggestions:

In D48587#1108227, @emaste wrote:

@guest-patmaddox has a proposal for this also, in https://github.com/freebsd/freebsd-src/pull/1552
PR: 283355

In D48587#1108230, @emaste wrote:

Note that we sort-of rely on install not validating -o and -g in -U/-M mode for some uses (see discussion in D48506).

Yes, I think this should be a feature and not a bug. -U and -o/-g without -M makes no sense.

In D48587#1108227, @emaste wrote:

@guest-patmaddox has a proposal for this also, in https://github.com/freebsd/freebsd-src/pull/1552
PR: 283355

I personally prefer to commit the fix and the test together, however I have been instructed in the past to do it separately. If this is still the best practice, I will split it upon commit.

In D48504#1107335, @emaste wrote:

In D48504#1107303, @jlduran wrote:

P.S.: Once this fix is committed, I plan on submitting it to NetBSD, given it is mtree related.

I've collaborated with @christos_netbsd.org on some changes for NetBSD in the past - he can maybe just pick this up once it lands.

Maybe I am missing something, but I still believe it should match uid_t/gid_t:

# touch /tmp/foo
# chown 4294967295:4294967295 /tmp/foo
# chown 18446744073709551615:18446744073709551615 /tmp/foo
chown: 18446744073709551615: illegal group name

I also think this fix is valid.
Regarding this particular implementation, as parseid() will just strtoul() the name, shouldn't the id be casted to (unsigned int) instead?:

install -U -M /dev/stdout -o -1 -g wheel /usr/bin/true /tmp
./tmp/true type=file uid=18446744073709551615 gname=wheel mode=0755 size=5120

P.S.: Once this fix is committed, I plan on submitting it to NetBSD, given it is mtree related.

It is fine by me, as long as it is also updated in:
https://cgit.freebsd.org/src/tree/tools/tools/git/hooks/prepare-commit-msg#n47
(and align all the trailers accordingly)

Is this the same as D47889?

Thank you!

In D47585#1090640, @markj wrote:

In D47585#1085259, @jlduran wrote:

This is the test I'm using for the fix in D47534.
As explained in that review, Jenkins alerted that it broke test:
https://ci.freebsd.org/view/Test/job/FreeBSD-main-amd64-test/lastCompletedBuild/testReport/junit/sys.net.routing.test_routing_l3/py/TestIfOps__test_change_prefix_route_inet_/

Do we understand why this test is failing? Is it a test bug or does this represent a real bug in the commit?

One thing I've noticed is that NetBSD moved the setting of groups/uid/gid earlier to inside notify() (https://github.com/NetBSD/src/commit/46b017828cee516770586497237aed6182b1decf). I think failing earlier is also a good move.

Also, I have yet to investigate if the mtree parsing under makefs is too strict (https://cgit.freebsd.org/src/tree/usr.sbin/makefs/mtree.c#n1004). But I'll check that once I have finished polishing building NanoBSD without root privileges (https://reviews.freebsd.org/D47653#1086475, https://www.youtube.com/watch?v=5qCaOMQ3ZnQ&t=2917s).

In D47722#1090635, @emaste wrote:

This is fine with me, but you could instead strip a leading / from TESTBASE if present, e.g. /usr/lib/debug/${TESTBASE:S,^/,,}. This will be robust against a possible TESTBASE without a leading slash.

• Remove bogus .debug directory

In D47668#1087469, @dfr wrote:

In D47668#1087193, @jlduran wrote:

Another +1.
Testing here using podman to inject metadata into containers, something similar to the original motivation.
Regarding the restrictions for reading the sysctl from inside the jail, I have no strong opinions, and fully trust your judgement (based on current names, most likely allow.read_meta, as I would interpret allow.metadata as being able to set metadata from inside the jail).

Is this jail metadata visible inside the jail? For podman, read-only host-visible metadata can be added as annotations on the container but these are not visible inside the container's jail.

Another +1.
Testing here using podman to inject metadata into containers, something similar to the original motivation.
Regarding the restrictions for reading the sysctl from inside the jail, I have no strong opinions, and fully trust your judgement (based on current names, most likely allow.read_meta, as I would interpret allow.metadata as being able to set metadata from inside the jail).