On Wed, Dec 19, 2012 at 07:08:57PM +0000, Christos Zoulas wrote:
> In article <20121219184352.GC38760%lor.one-eyed-alien.net@localhost>,
> Brooks Davis <brooks%freebsd.org@localhost> wrote:
> >
> >Here's an improved patch that disables iflag and documents this
> >behavior. It also adds a COMPATIBILITY section which is incomplete
> >pending resolution of the rest of the issues.
> >
> >
> >/* FreeBSD compat, remove after 2018. */
> >
> >Index: mtree.c
> >===================================================================
> >--- mtree.c (revision 244436)
> >+++ mtree.c (working copy)
> >@@ -236,6 +236,12 @@
> > if ((cflag && Cflag) || (cflag && Dflag) || (Cflag && Dflag))
> > mtree_err("-c, -C and -D flags are mutually exclusive");
> >
> >+ if (cflag && iflag) {
> >+ warnx("-c and -i specified, setting -j and unsetting -i");
> >+ iflag = 0;
> >+ jflag = 1;
> >+ }
> >+
> > if (iflag && mflag)
> > mtree_err("-i and -m flags are mutually exclusive");
>
> Yes, that is fine, but read on.
>
> >Index: mtree.8
> >===================================================================
> >--- mtree.8 (revision 244436)
> >+++ mtree.8 (working copy)
> >@@ -166,6 +166,16 @@
> > If no inclusion list is provided, the default is to display all files.
> > .It Fl i
> > If specified, set the schg and/or sappnd flags.
> >+For compatibility with
> >+.Fx
> >+if the
> >+.Fl c
> >+and
> >+.Fl i
> >+option are both passed,
> >+then the
> >+.Fl j
> >+option is implied and a warning is emitted.
> > .It Fl j
> > Indent the output 4 spaces each time a directory level is descended when
> > creating a specification with the
> >@@ -690,6 +700,37 @@
> > or
> > .Fl u
> > to create directory hierarchies for, for example, distributions.
> >+.Sh COMPATIBILITY
> >+This version of the
> >+.Nm utility differs from the version in
> >+.Fx 9
> >+and earlier in several ways:
> >+.Bl -bullet -compact
> >+.It
> >+When the
> >+.Fl c
> >+option is specified a final ``..'' is not emitted to match the top
> >+directory.
> >+Files made to match by appending ``..\\n\\n''.
> >+.It
> >+The
> >+.Fl U
> >+and
> >+.Fl u
> >+options do not set the modification time or the schg and/or sappnd
> >+file flags unless the
> >+.Fl t
> >+and
> >+.Fl i
> >+options are also passed.
> >+.It
> >+The
> >+.Fl j
> >+option is equivalent to the
> >+.Fx
> >+.Fl i
> >+option and should be used in its place.
> >+.El
> > .Sh SEE ALSO
> > .Xr chflags 1 ,
> > .Xr chgrp 1 ,
>
> The more I read this man page, the more difficult it is for me to
> understand. I think we are trying to achieve both compatibility
> with the current versions of mtree, and make both the NetBSD
> and FreeBSD versions similar/identical, while keeping the more
> rational and informative output.
>
> These are all conflicting goals and we are introducing extra
> complexity to achieve them. This can cannibalize the future
> maintenability and usability of the utility.
>
> Why don't we do something like have a global flag/environment option
> to preserve the current behavior, and then design the new behavior
> as we want it to be without being constrained by compatibility?
> Then at least hopefully we can keep the compat code in one place,
> and remove them after the next release. Or even do it based on the
> command name (omtree).
I'd be OK with a flag to enable compatibility modes. Having a freebsd9
mode would make it easy to preserve output formatting at to provide
something to check to determine if we want to enable things like the -c
-i hack above.
The more I think about this the more I like it. The only downside I see
is that we may significantly increase the complexity of some of the
output cases.
As a strawman, how about an option -F <flavor> with initial values of
"freebsd9", "netbsd6", and "mtree"? I'd be happy to implement the basics.
One related question that occured to me: do we want an option to
suppress warnings about option compatibility shims? I think that's only
potentially relevant for FreeBSD.
I'll respond to the rest of this message on the assumption that we'll go
forward with something like that.
> >Hmm, that's true. The final .. looks more complete in -j output, but is
> >basically a no op in practice and trivial to add after the fact which
> >leads me to think I should just document the difference and move on.
> >
> >One compromise approach might be to enable it when -j is specified since
> >that's a new option on NetBSD. That could be accomplished as follows:
> >
> >Index: create.c
> >===================================================================
> >RCS file: /cvsroot/src/usr.sbin/mtree/create.c,v
> >retrieving revision 1.67
> >diff -u -r1.67 create.c
> >--- create.c 15 Dec 2012 01:24:40 -0000 1.67
> >+++ create.c 19 Dec 2012 18:20:24 -0000
> >@@ -143,12 +143,12 @@
> > statf(indent, p);
> > break;
> > case FTS_DP:
> >- if (p->fts_level > 0) {
> >+ if (p->fts_level > 0)
> > if (!nflag)
> > printf("%*s# %s\n", indent, "",
> > p->fts_path);
> >+ if (p->fts_level > 0 || jflag)
> > printf("%*s..\n\n", indent, "");
> >- }
> > break;
> > case FTS_DNR:
> > case FTS_ERR:
> >
>
> I don't like this much because it is weird to have the output change because
> an unrelated option is set.
Makes sense, this should clearly be handled as a legacy FreeBSD specific
option.
> >> >> - size keywords are printed for all file types.
> >
> >If you're willing to do this it would be helpful. I know I will have to
> >enable it to make this version acceptable on FreeBSD. Otherwise every
> >non-file line will change in the default case. Obviously the reverse
> >is that if you do make this change the files of NetBSD users change
> >instead. That being said, the time=%d.%09d bugfix means you already
> >have done that in the default case so now is as good a time as any to
> >make major changes.
>
> I am ok with that.
With flavor support size is easy to handle.
> >That could work. What about the /set statement? I think I'll probably
> >need to change that on FreeBSD to avoid churning every line. The
> >type=file seems obviously wrong give that we aren't printing output for
> >files.
>
> Sounds good to me.
Ok, how about a -N option to suppress extra newlines and making /set
type=dir the default in the -d case going forward with a netbsd6 compat
option?
One other format note, I realized earlier today one other format
difference that I think is behind the fact that encoding glob characters
didn't work as expected. In FreeBSD we encode whitespace and glob
characters with octal encoding rather than C-style encoding so all of
them get reexpanded.
-- Brooks
Attachment:
pgp2yqK47lru7.pgp
Description: PGP signature