Re: mtree(1) recent POLA violation

On Tue, Jan 6, 2026 at 9:08 PM Xin LI <delphij@gmail.com> wrote:
>
>
>
> On Tue, Jan 6, 2026 at 3:47 PM Jose Luis Duran <jlduran@freebsd.org> wrote:
>>
>> On Tue, Jan 6, 2026 at 8:37 PM Xin LI <delphij@gmail.com> wrote:
>> >
>> > (Adding ngie@ who reported PR 219467 and Christos for visibility)
>> >
>> > On Tue, Jan 6, 2026 at 3:05 PM Jose Luis Duran <jlduran@freebsd.org> wrote:
>> >>
>> >> On Tue, Jan 6, 2026 at 7:37 PM Gleb Smirnoff <glebius@freebsd.org> 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.

-- 
Jose Luis Duran