On Tue, Jan 6, 2026 at 4:21 PM Jose Luis Duran <[email protected]> wrote:
> On Tue, Jan 6, 2026 at 9:08 PM Xin LI <[email protected]> wrote: > > > > > > > > On Tue, Jan 6, 2026 at 3:47 PM Jose Luis Duran <[email protected]> > wrote: > >> > >> On Tue, Jan 6, 2026 at 8:37 PM Xin LI <[email protected]> wrote: > >> > > >> > (Adding ngie@ who reported PR 219467 and Christos for visibility) > >> > > >> > On Tue, Jan 6, 2026 at 3:05 PM Jose Luis Duran <[email protected]> > wrote: > >> >> > >> >> On Tue, Jan 6, 2026 at 7:37 PM Gleb Smirnoff <[email protected]> > wrote: > >> >> > > >> >> > Hi, > >> >> > > >> >> > the recent mtree(1) import from NetBSD brought one change, that is > a POLA > >> >> > violation and I would also question if the new behavior is desired. > >> >> > >> >> The change stems from: > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219467 > >> >> > >> >> > Before the import 'mtree -c -R all' would leave 'type=' keywords, > despite '-R > >> >> > all' asks for removing all keywords. The 'type=' is special, > since this > >> >> > keyword is required to reconstruct a new spec. > >> >> > > >> >> > In other words before the import this was working: > >> >> > > >> >> > mtree -c -R all | mtree -C > >> >> > > >> >> > Now this is broken. The above was standard idiom to compare > installed to tree > >> >> > to a specification. Now the correct syntax to get the same > behavior is this: > >> >> > > >> >> > mtree -c -k type | mtree -C > >> >> > > >> >> > I'll let other to decide do we want to fix this POLA violation or > not. At least > >> >> > this needs to be recorded in Release notes. > >> >> > >> >> Right, according to the manual page: > >> >> > >> >> -k <keywords>: Use the "type" keyword plus the specified (whitespace > >> >> or comma separated) keywords instead of the current set of keywords. > >> >> If "all" is specified, use all of the other keywords. If the "type" > >> >> keyword is not desired, suppress it with "-R type". > >> >> -R <keywords>: Remove the specified (whitespace or comma separated) > >> >> keywords from the current set of keywords. If "all" is specified, > >> >> remove all of the other keywords. > >> >> > >> >> So, the previous behavior was bugged. It now does what it says it > should. > >> > > >> > > >> > If we look at when the keyword feature was initially implemented > (CSRG r51884, 34 years ago), it's quite clear that "type" was special: > F_TYPE is always set regardless of what's specified with '-k' (mtree.c), > and in create.c the flag is the only one not being checked. IMHO "type" > represents a material difference in a file hierarchy specification, and > should always be present for non-plain files. > >> > > >> > It has been there for 34 years and legitimate users evidently rely on > this feature and the historical behavior should not be considered a bug. I > think we should restore the historical behavior and amend the documentation > to reflect it instead. > >> > >> I'm not opposed to reverting it, if we also change (something along the > lines): > >> > >> 1. Remove: "If the type keyword is not desired, suppress it with -R > >> type." from the "-k" flag description. > > > > > > Yes I think NetBSD mtree.8,v 1.44 ( > https://mail-index.netbsd.org/source-changes/2006/09/12/0034.html ) > should be reverted. > > > >> > >> 2. Add: "If 'all' is specified, remove all of the other keywords with > >> the exception of 'type'." > > > > > > In fact the current wording already mentions it: > > > > Use the type keyword plus the specified (whitespace or comma separated) > keywords instead of the current set of keywords. If ‘all’ is specified, use > all of the other keywords. > > > > I'd probably change the '-k' part to: > > > > ===== > > .It Fl k Ar keywords > > Use the mandatory > > .Sy type > > keyword plus the specified (whitespace or comma separated) > > .Ar keywords > > to replace the current set of keywords. > > If > > .Ql all > > is specified, use all of the available keywords. > > ===== > > > > and the '-R' part to: > > > > ===== > > .It Fl R Ar keywords > > Remove the specified (whitespace or comma separated) keywords from the > current > > set of keywords. > > The > > .Sy type > > keyword is mandatory and is always retained. > > If > > .Ql all > > is specified, remove all keywords except > > .Sy type . > > ===== > > to make it more clear. > > > > That would render as: > > > > -k keywords > > Use the mandatory type keyword plus the specified > (whitespace > > or comma separated) keywords to replace the current set > of > > keywords. If ‘all’ is specified, use all of the > available > > keywords. > > > > [...] > > > > -R keywords > > Remove the specified (whitespace or comma separated) > keywords > > from the current set of keywords. The type keyword is > > mandatory and is always retained. If ‘all’ is > specified, > > remove all keywords except type. > > > > > > as attached. > > > > > > > >> > >> > >> > Cheers, > >> > >> > >> Regards, > >> > >> -- > >> Jose Luis Duran > > > Yes, number 2 was for the -R flag, as you noted. > > The code "fix" is to revert: > > https://github.com/NetBSD/src/commit/2163af1bb7ee561f0ab2251ddd2693a84909a7db > > Let's wait for ngie, otherwise, I'll submit the patch to GNATS. > Thanks! There is no rush, let's wait for ngie to chime in in case my reasonaing wasn't convincing enough :) Cheers,
