Re: mtree(1) recent POLA violation

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,