Date: Tue, 6 Jan 2026 16:24:18 -0800 From: Xin LI <delphij@gmail.com> To: jlduran@freebsd.org Cc: Gleb Smirnoff <glebius@freebsd.org>, current@freebsd.org, christos@netbsd.org, Ngie Cooper <ngie@freebsd.org> Subject: Re: mtree(1) recent POLA violation Message-ID: <CAGMYy3seKFB8zgxb1PsKWYemJETkf62Gqb6AeWY5puP-oz4GJg@mail.gmail.com> In-Reply-To: <CAPwQLceU0pxzruUbeBbUgyr6%2B=%2BijNsbdhDHDWoXeGy-96ex2w@mail.gmail.com> References: <aV2OtWy0JknNGC1b@cell.glebi.us> <CAPwQLcen1C4YH6nBdsip8dzme3bQHVVLJ%2BkgdtJK83ffRjA0sg@mail.gmail.com> <CAGMYy3vJ9jvkRSdWCWc4d4B5ToyoBUO1cRQXfRLKVwTYGQokBA@mail.gmail.com> <CAPwQLcdt%2BKgOygDVrBwuBvv_3U=7xLdU2g6ooc2zrj0OyJfaoQ@mail.gmail.com> <CAGMYy3vc68TQ54eD2wYw%2BwgenDSDKvRNiSOgFfk1ObFP_ebqcQ@mail.gmail.com> <CAPwQLceU0pxzruUbeBbUgyr6%2B=%2BijNsbdhDHDWoXeGy-96ex2w@mail.gmail.com>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] On Tue, Jan 6, 2026 at 4:21 PM Jose Luis Duran <jlduran@freebsd.org> wrote: > 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. > Thanks! There is no rush, let's wait for ngie to chime in in case my reasonaing wasn't convincing enough :) Cheers, [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Jan 6, 2026 at 4:21 PM Jose Luis Duran <<a href="mailto:jlduran@freebsd.org">jlduran@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Tue, Jan 6, 2026 at 9:08 PM Xin LI <<a href="mailto:delphij@gmail.com" target="_blank">delphij@gmail.com</a>> wrote:<br> ><br> ><br> ><br> > On Tue, Jan 6, 2026 at 3:47 PM Jose Luis Duran <<a href="mailto:jlduran@freebsd.org" target="_blank">jlduran@freebsd.org</a>> wrote:<br> >><br> >> On Tue, Jan 6, 2026 at 8:37 PM Xin LI <<a href="mailto:delphij@gmail.com" target="_blank">delphij@gmail.com</a>> wrote:<br> >> ><br> >> > (Adding ngie@ who reported PR 219467 and Christos for visibility)<br> >> ><br> >> > On Tue, Jan 6, 2026 at 3:05 PM Jose Luis Duran <<a href="mailto:jlduran@freebsd.org" target="_blank">jlduran@freebsd.org</a>> wrote:<br> >> >><br> >> >> On Tue, Jan 6, 2026 at 7:37 PM Gleb Smirnoff <<a href="mailto:glebius@freebsd.org" target="_blank">glebius@freebsd.org</a>> wrote:<br> >> >> ><br> >> >> > Hi,<br> >> >> ><br> >> >> > the recent mtree(1) import from NetBSD brought one change, that is a POLA<br> >> >> > violation and I would also question if the new behavior is desired.<br> >> >><br> >> >> The change stems from: <a href="https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219467" rel="noreferrer" target="_blank">https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=219467</a><br>; >> >><br> >> >> > Before the import 'mtree -c -R all' would leave 'type=' keywords, despite '-R<br> >> >> > all' asks for removing all keywords. The 'type=' is special, since this<br> >> >> > keyword is required to reconstruct a new spec.<br> >> >> ><br> >> >> > In other words before the import this was working:<br> >> >> ><br> >> >> > mtree -c -R all | mtree -C<br> >> >> ><br> >> >> > Now this is broken. The above was standard idiom to compare installed to tree<br> >> >> > to a specification. Now the correct syntax to get the same behavior is this:<br> >> >> ><br> >> >> > mtree -c -k type | mtree -C<br> >> >> ><br> >> >> > I'll let other to decide do we want to fix this POLA violation or not. At least<br> >> >> > this needs to be recorded in Release notes.<br> >> >><br> >> >> Right, according to the manual page:<br> >> >><br> >> >> -k <keywords>: Use the "type" keyword plus the specified (whitespace<br> >> >> or comma separated) keywords instead of the current set of keywords.<br> >> >> If "all" is specified, use all of the other keywords. If the "type"<br> >> >> keyword is not desired, suppress it with "-R type".<br> >> >> -R <keywords>: Remove the specified (whitespace or comma separated)<br> >> >> keywords from the current set of keywords. If "all" is specified,<br> >> >> remove all of the other keywords.<br> >> >><br> >> >> So, the previous behavior was bugged. It now does what it says it should.<br> >> ><br> >> ><br> >> > 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.<br> >> ><br> >> > 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.<br> >><br> >> I'm not opposed to reverting it, if we also change (something along the lines):<br> >><br> >> 1. Remove: "If the type keyword is not desired, suppress it with -R<br> >> type." from the "-k" flag description.<br> ><br> ><br> > Yes I think NetBSD mtree.8,v 1.44 ( <a href="https://mail-index.netbsd.org/source-changes/2006/09/12/0034.html" rel="noreferrer" target="_blank">https://mail-index.netbsd.org/source-changes/2006/09/12/0034.html</a>; ) should be reverted.<br> ><br> >><br> >> 2. Add: "If 'all' is specified, remove all of the other keywords with<br> >> the exception of 'type'."<br> ><br> ><br> > In fact the current wording already mentions it:<br> ><br> > 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.<br> ><br> > I'd probably change the '-k' part to:<br> ><br> > =====<br> > .It Fl k Ar keywords<br> > Use the mandatory<br> > .Sy type<br> > keyword plus the specified (whitespace or comma separated)<br> > .Ar keywords<br> > to replace the current set of keywords.<br> > If<br> > .Ql all<br> > is specified, use all of the available keywords.<br> > =====<br> ><br> > and the '-R' part to:<br> ><br> > =====<br> > .It Fl R Ar keywords<br> > Remove the specified (whitespace or comma separated) keywords from the current<br> > set of keywords.<br> > The<br> > .Sy type<br> > keyword is mandatory and is always retained.<br> > If<br> > .Ql all<br> > is specified, remove all keywords except<br> > .Sy type .<br> > =====<br> > to make it more clear.<br> ><br> > That would render as:<br> ><br> > -k keywords<br> > Use the mandatory type keyword plus the specified (whitespace<br> > or comma separated) keywords to replace the current set of<br> > keywords. If ‘all’ is specified, use all of the available<br> > keywords.<br> ><br> > [...]<br> ><br> > -R keywords<br> > Remove the specified (whitespace or comma separated) keywords<br> > from the current set of keywords. The type keyword is<br> > mandatory and is always retained. If ‘all’ is specified,<br> > remove all keywords except type.<br> ><br> ><br> > as attached.<br> ><br> ><br> ><br> >><br> >><br> >> > Cheers,<br> >><br> >><br> >> Regards,<br> >><br> >> --<br> >> Jose Luis Duran<br> <br> <br> Yes, number 2 was for the -R flag, as you noted.<br> <br> The code "fix" is to revert:<br> <a href="https://github.com/NetBSD/src/commit/2163af1bb7ee561f0ab2251ddd2693a84909a7db" rel="noreferrer" target="_blank">https://github.com/NetBSD/src/commit/2163af1bb7ee561f0ab2251ddd2693a84909a7db</a><br>; <br> Let's wait for ngie, otherwise, I'll submit the patch to GNATS.<br></blockquote><div><br></div><div class="gmail_default" style="font-family:monospace,monospace">Thanks! There is no rush, let's wait for ngie to chime in in case my reasonaing wasn't convincing enough :)</div><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_default" style="font-family:monospace,monospace">Cheers,</div></div></div>home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGMYy3seKFB8zgxb1PsKWYemJETkf62Gqb6AeWY5puP-oz4GJg>