On Wed, Dec 19, 2012 at 02:52:57PM -0600, Brooks Davis wrote:
> 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.
I've gone ahead and implemented this along with a -b option to suppress
extra blank lines so I could implement FreeBSD style -d output.
> 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.
I elected to switch the non-netbsd6 case to follow FreeBSD. It's not
entirely ideal in that it's less readable, but the potential hazards to
running in update mode when the paths contain globs are probably
sufficient to make it the right choice.
-- Brooks
Index: mtree.8
===================================================================
--- mtree.8 (revision 244449)
+++ mtree.8 (working copy)
@@ -64,9 +64,10 @@
.Nd map a directory hierarchy
.Sh SYNOPSIS
.Nm
-.Op Fl CcDdejLlMnPqrStUuWx
+.Op Fl bCcDdejLlMnPqrStUuWx
.Op Fl i | Fl m
.Op Fl E Ar tags
+.Op Fl F Ar flavor
.Op Fl f Ar spec
.Op Fl I Ar tags
.Op Fl K Ar keywords
@@ -92,6 +93,8 @@
.Pp
The options are as follows:
.Bl -tag -width Xxxexcludexfilexx
+.It Fl b
+Suppress blank lines before entering and after exiting directories.
.It Fl C
Convert a specification into
a format that's easier to parse with various tools.
@@ -140,6 +143,29 @@
.It Fl e
Don't complain about files that are in the file hierarchy, but not in the
specification.
+.It Fl F Ar flavor
+Set the compatibilty flavor of the
+.Nm
+utility.
+The
+.Ar flavor
+can be one of
+.Sy mtree ,
+.Sy freebsd9 ,
+or
+.Sy netbsd6 .
+The default is
+.Sy mtree .
+The
+.Sy freebsd9
+and
+.Sy netbsd6
+flavors attempt to preserve output compatiblity and command line optio
+backward compatibility with
+.Fx 9
+and
+.Nx 6
+respectively.
.It Fl f Ar spec
Read the specification from
.Ar file ,
@@ -722,6 +748,35 @@
.Fl i
option and should be used in its place.
.El
+.Sh COMPATIBILITY
+The compatibility shims provided by the
+.Fl F
+option are incomplete by design.
+Known limititations are described below.
+.Pp
+The
+.Sy freebsd9
+flavor retains the default handling of lookup failures for the
+.Sy uname
+and
+.Sy group
+keywords by replacing them with appropriate
+.Sy uid
+and
+.Sy gid
+keywords rather than failing and reporting an error.
+The related
+.Fl w
+flag is a no-op rather than causing a warning to be printed and no
+keyword to be emitted.
+The latter behavior is not emulated as it is potentially dangerous in
+the face of /set statements.
+.Pp
+The
+.Sy netbsd6
+flavor does not replicate the historical bug that reported time as
+seconds.nanoseconds without zero padding nanosecond values less than
+100000000.
.Sh SEE ALSO
.Xr chflags 1 ,
.Xr chgrp 1 ,
Index: create.c
===================================================================
--- create.c (revision 244449)
+++ create.c (working copy)
@@ -136,18 +136,22 @@
}
switch(p->fts_info) {
case FTS_D:
- printf("\n");
+ if (!bflag)
+ printf("\n");
if (!nflag)
printf("# %s\n", p->fts_path);
statd(t, p, &uid, &gid, &mode, &flags);
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);
- printf("%*s..\n\n", indent, "");
+ if (p->fts_level > 0 || flavor == F_FREEBSD9) {
+ printf("%*s..\n", indent, "");
+ if (!bflag)
+ printf("\n");
}
break;
case FTS_DNR:
@@ -186,7 +190,7 @@
else
offset += printf("%*s", (INDENTNAMELEN + indent) - offset, "");
- if (!S_ISREG(p->fts_statp->st_mode))
+ if (!S_ISREG(p->fts_statp->st_mode) && (flavor == F_NETBSD6 || !dflag))
output(indent, &offset, "type=%s",
inotype(p->fts_statp->st_mode));
if (keys & (F_UID | F_UNAME) && p->fts_statp->st_uid != uid) {
@@ -212,7 +216,8 @@
(long long)p->fts_statp->st_rdev);
if (keys & F_NLINK && p->fts_statp->st_nlink != 1)
output(indent, &offset, "nlink=%u", p->fts_statp->st_nlink);
- if (keys & F_SIZE && S_ISREG(p->fts_statp->st_mode))
+ if (keys & F_SIZE &&
+ (flavor != F_NETBSD6 || S_ISREG(p->fts_statp->st_mode)))
output(indent, &offset, "size=%lld",
(long long)p->fts_statp->st_size);
if (keys & F_TIME)
@@ -349,29 +354,32 @@
maxuid = maxgid = maxmode = maxflags = 0;
for (; p; p = p->fts_link) {
- smode = p->fts_statp->st_mode & MBITS;
- if (smode < MTREE_MAXMODE && ++m[smode] > maxmode) {
- savemode = smode;
- maxmode = m[smode];
- }
- sgid = p->fts_statp->st_gid;
- if (sgid < MTREE_MAXGID && ++g[sgid] > maxgid) {
- savegid = sgid;
- maxgid = g[sgid];
- }
- suid = p->fts_statp->st_uid;
- if (suid < MTREE_MAXUID && ++u[suid] > maxuid) {
- saveuid = suid;
- maxuid = u[suid];
- }
+ if (flavor == F_NETBSD6 || !dflag ||
+ (dflag && S_ISDIR(p->fts_statp->st_mode))) {
+ smode = p->fts_statp->st_mode & MBITS;
+ if (smode < MTREE_MAXMODE && ++m[smode] > maxmode) {
+ savemode = smode;
+ maxmode = m[smode];
+ }
+ sgid = p->fts_statp->st_gid;
+ if (sgid < MTREE_MAXGID && ++g[sgid] > maxgid) {
+ savegid = sgid;
+ maxgid = g[sgid];
+ }
+ suid = p->fts_statp->st_uid;
+ if (suid < MTREE_MAXUID && ++u[suid] > maxuid) {
+ saveuid = suid;
+ maxuid = u[suid];
+ }
#if HAVE_STRUCT_STAT_ST_FLAGS
- sflags = FLAGS2INDEX(p->fts_statp->st_flags);
- if (sflags < MTREE_MAXFLAGS && ++f[sflags] > maxflags) {
- saveflags = p->fts_statp->st_flags;
- maxflags = f[sflags];
+ sflags = FLAGS2INDEX(p->fts_statp->st_flags);
+ if (sflags < MTREE_MAXFLAGS && ++f[sflags] > maxflags) {
+ saveflags = p->fts_statp->st_flags;
+ maxflags = f[sflags];
+ }
+#endif
}
-#endif
}
/*
* If the /set record is the same as the last one we do not need to
@@ -384,7 +392,10 @@
((keys & F_FLAGS) && (*pflags != saveflags)) ||
first) {
first = 0;
- printf("/set type=file");
+ if (flavor != F_NETBSD6 && dflag)
+ printf("/set type=dir");
+ else
+ printf("/set type=file");
if (keys & (F_UID | F_UNAME)) {
if (keys & F_UNAME &&
(name = user_from_uid(saveuid, 1)) != NULL)
Index: extern.h
===================================================================
--- extern.h (revision 244449)
+++ extern.h (working copy)
@@ -52,6 +52,12 @@
#define MAXHOSTNAMELEN 256
#endif
+enum flavor {
+ F_MTREE,
+ F_FREEBSD9,
+ F_NETBSD6
+};
+
void addtag(slist_t *, char *);
int check_excludes(const char *, const char *);
int compare(NODE *, FTSENT *);
@@ -69,10 +75,11 @@
const char *rlink(const char *);
int verify(FILE *);
-extern int dflag, eflag, iflag, jflag, lflag, mflag,
+extern int bflag, dflag, eflag, iflag, jflag, lflag, mflag,
nflag, qflag, rflag, sflag, tflag, uflag;
extern int mtree_Mflag, mtree_Sflag, mtree_Wflag;
extern size_t mtree_lineno;
+extern enum flavor flavor;
extern u_int32_t crc_total;
extern int ftsoptions, keys;
extern char fullpath[];
Index: mtree.c
===================================================================
--- mtree.c (revision 244449)
+++ mtree.c (working copy)
@@ -59,16 +59,27 @@
#include "extern.h"
int ftsoptions = FTS_PHYSICAL;
-int cflag, Cflag, dflag, Dflag, eflag, iflag, jflag, lflag, mflag,
- nflag, qflag, rflag, sflag, tflag, uflag, Uflag;
+int bflag, cflag, Cflag, dflag, Dflag, eflag, iflag, jflag, lflag, mflag,
+ nflag, qflag, rflag, sflag, tflag, uflag, Uflag, wflag;
char fullpath[MAXPATHLEN];
+enum flavor flavor = F_MTREE;
+static struct {
+ enum flavor flavor;
+ const char name[9];
+} flavors[] = {
+ {F_MTREE, "mtree"},
+ {F_FREEBSD9, "freebsd9"},
+ {F_NETBSD6, "netbsd6"},
+};
+
__dead static void usage(void);
int
main(int argc, char **argv)
{
int ch, status;
+ uint i;
char *dir, *p;
FILE *spec1, *spec2;
@@ -80,9 +91,12 @@
spec2 = NULL;
while ((ch = getopt(argc, argv,
- "cCdDeE:f:I:ijk:K:lLmMnN:p:PqrR:s:StuUWxX:"))
+ "bcCdDeE:f:F:I:ijk:K:lLmMnN:p:PqrR:s:StuUwWxX:"))
!= -1) {
switch((char)ch) {
+ case 'b':
+ bflag = 1;
+ break;
case 'c':
cflag = 1;
break;
@@ -115,6 +129,15 @@
} else
usage();
break;
+ case 'F':
+ for (i = 0; i < __arraycount(flavors); i++)
+ if (strcmp(optarg, flavors[i].name) == 0) {
+ flavor = flavors[i].flavor;
+ break;
+ }
+ if (i == __arraycount(flavors))
+ usage();
+ break;
case 'i':
iflag = 1;
break;
@@ -193,6 +216,9 @@
case 'U':
Uflag = uflag = 1;
break;
+ case 'w':
+ wflag = 1;
+ break;
case 'W':
mtree_Wflag = 1;
break;
@@ -213,6 +239,36 @@
if (argc)
usage();
+ switch (flavor) {
+ case F_FREEBSD9:
+ if (cflag && iflag) {
+ warnx("-c and -i passed, replacing -i with -j for "
+ "FreeBSD compatibility");
+ iflag = 0;
+ jflag = 1;
+ }
+ if (dflag && !bflag) {
+ warnx("Adding -b to -d for FreeBSD compatibility");
+ bflag = 1;
+ }
+ if (uflag && !iflag) {
+ warnx("Adding -i to -%c for FreeBSD compatibility",
+ Uflag ? 'U' : 'u');
+ iflag = 1;
+ }
+ if (uflag && !tflag) {
+ warnx("Adding -t to -%c for FreeBSD compatibility",
+ Uflag ? 'U' : 'u');
+ tflag = 1;
+ }
+ if (wflag)
+ warnx("The -w flag is a no-op");
+ break;
+ default:
+ if (wflag)
+ usage();
+ }
+
if (spec2 && (cflag || Cflag || Dflag))
mtree_err("Double -f, -c, -C and -D flags are mutually "
"exclusive");
@@ -261,12 +317,18 @@
static void
usage(void)
{
+ uint i;
fprintf(stderr,
- "usage: %s [-CcDdejLlMnPqrStUuWx] [-i|-m] [-E tags]\n"
+ "usage: %s [-bCcDdejLlMnPqrStUuWx] [-i|-m] [-E tags]\n"
"\t\t[-f spec] [-f spec]\n"
"\t\t[-I tags] [-K keywords] [-k keywords] [-N dbdir] [-p path]\n"
- "\t\t[-R keywords] [-s seed] [-X exclude-file]\n",
+ "\t\t[-R keywords] [-s seed] [-X exclude-file]\n"
+ "\t\t[-F flavor]\n",
getprogname());
+ fprintf(stderr, "\nflavors:");
+ for (i = 0; i < __arraycount(flavors); i++)
+ fprintf(stderr, " %s", flavors[i].name);
+ fprintf(stderr, "\n");
exit(1);
}
Index: spec.c
===================================================================
--- spec.c (revision 244449)
+++ spec.c (working copy)
@@ -415,11 +415,15 @@
char *
vispath(const char *path)
{
- const char extra[] = { ' ', '\t', '\n', '\\', '#', '*', '?', '[',
- '#', '\0' };
+ const char extra[] = { ' ', '\t', '\n', '\\', '\0' };
+ const char extra_glob[] = { ' ', '\t', '\n', '\\', '#', '*', '?',
+ '[', '\0' };
static char pathbuf[4*MAXPATHLEN + 1];
- strsvis(pathbuf, path, VIS_CSTYLE, extra);
+ if (flavor == F_NETBSD6)
+ strsvis(pathbuf, path, VIS_CSTYLE, extra);
+ else
+ strsvis(pathbuf, path, VIS_OCTAL, extra_glob);
return(pathbuf);
}
Attachment:
pgphpQkPoxO62.pgp
Description: PGP signature