Dear Mohamed, On Wed, Feb 22, 2023 at 05:42:10AM +0000, Mohamed Bukhris wrote: > This patch adds the -m/--prume-empty-dirs option to openrsync > while keeping said feature compatible with rsync > this avoids the 27 -> 31 protocol mismatch error by not sharing the -m option > to remote > This was tested locally (openrsync -> openrsync) and remotely (openrsync -> > rsync)
I have a few style remarks, see http://man.openbsd.org/style for full overview of how the code should be formatted to make review & understanding easier within the context of the OpenBSD project. Review is slightly easier if you use 'cvs diff -uNp' to generate the diff. See https://www.openbsd.org/faq/faq5.html#Diff > diff -ura ../origsync/extern.h ./extern.h > --- ../origsync/extern.h 2023-02-21 21:43:18.871417908 +0100 > +++ ./extern.h 2023-02-21 18:26:12.176316267 +0100 > @@ -134,6 +134,7 @@ > int server; /* --server */ > int recursive; /* -r */ > int dry_run; /* -n */ > + int prune_empty_dirs; /* -m */ > int preserve_times; /* -t */ > int preserve_perms; /* -p */ > int preserve_links; /* -l */ > diff -ura ../origsync/flist.c ./flist.c > --- ../origsync/flist.c 2023-02-21 21:43:07.778145448 +0100 > +++ ./flist.c 2023-02-22 06:09:51.097975517 +0100 > @@ -907,6 +907,36 @@ > continue; > } > > + /* > + * If -m (prune empty dirs) is enabled, create a new fts > + * to independently traverse directories at once and determine > + * whether we are dealing with a hierarchy of empty > + * directories, if so, skip. > + */ > + > + if (sess->opts->prune_empty_dirs && ent->fts_info == FTS_D){ Add a space between ) and { > + char *prune_cargv[2]; > + prune_cargv[0] = ent->fts_name; > + prune_cargv[1] = NULL; > + FTS *prunefts; Declare FTS *prunefts at the top of flist_gen_dirent() with the rest. > + if ((prunefts = fts_open(prune_cargv, > FTS_PHYSICAL, NULL)) == NULL) { All code should fit in 80 columns. Add a newline after 'prune_cargv,' and indent FTS_PHYSICAL with 4 spaces to make it distinct from the if (). > + ERR("fts_open"); > + return 0; > + } > + FTSENT *prunent; > + int empty_chain = 1; Please initiatize variables at the top of flist_gen_dirent(). > + while ((prunent = fts_read(prunefts)) != NULL) { > + if (prunent->fts_info != FTS_D && > prunent->fts_info != FTS_DP){ Above line is too long. Add a space between ) and { > + empty_chain = 0; > + break; > + } > + } > + if (empty_chain){ > + continue; > + } Add a space between ) and {, and as 'continue' is the only statement inside this if-block, you can write it like so: if (empty_chain) continue; > + fts_close(prunefts); > + } > + > /* We don't allow symlinks without -l. */ > > assert(ent->fts_statp != NULL); > diff -ura ../origsync/main.c ./main.c > --- ../origsync/main.c 2023-02-21 21:43:10.861461862 +0100 > +++ ./main.c 2023-02-22 05:21:15.047310523 +0100 > @@ -340,6 +340,7 @@ > { "verbose", no_argument, &verbose, 1 }, > { "no-verbose", no_argument, &verbose, 0 }, > { "version", no_argument, NULL, OP_VERSION }, > + { "prune-empty-dirs", no_argument, &opts.prune_empty_dirs, 1 }, Align it using spaces like the other lines inside struct option lopts[] Use semi-alphabetical ordering, move the line to under "port". > { NULL, 0, NULL, 0 } > }; > > @@ -362,7 +363,7 @@ > > opts.max_size = opts.min_size = -1; > > - while ((c = getopt_long(argc, argv, "Dae:ghlnoprtvxz", lopts, &lidx)) > + while ((c = getopt_long(argc, argv, "Dae:ghlnoprtvxzm", lopts, &lidx)) The optstring argument should be ordered, add 'm' after l' instead. > != -1) { > switch (c) { > case 'D': > @@ -382,6 +383,9 @@ > case 'e': > opts.ssh_prog = optarg; > break; > + case 'm': > + opts.prune_empty_dirs = 1; > + break; > case 'g': > opts.preserve_gids = 1; > break; > @@ -633,7 +637,7 @@ > exit(rc); > usage: > fprintf(stderr, "usage: %s" > - " [-aDglnoprtvx] [-e program] [--address=sourceaddr]\n" > + " [-aDglnoprtvxm] [-e program] [--address=sourceaddr]\n" add 'm' after 'l' instead > "\t[--contimeout=seconds] [--compare-dest=dir] [--del] > [--exclude]\n" > "\t[--exclude-from=file] [--include] [--include-from=file]\n" > "\t[--no-motd] [--numeric-ids] [--port=portnumber]\n" > diff -ura ../origsync/rsync.1 ./rsync.1 > --- ../origsync/rsync.1 2023-02-21 21:43:19.451414725 +0100 > +++ ./rsync.1 2023-02-22 05:45:26.459345977 +0100 > @@ -220,6 +220,8 @@ > If this option is repeated, all mount point directories from the copy are > omitted. > Otherwise, it includes an empty directory at each mount point it encounters. > +.It Fl m , -prune-empty-dirs > +Prune empty directory chains from the file list. > .It Fl -version > Print version and exit. > .El Move it to above '-dry-run' Thanks! Kind regards, Job
