Any comments, or can this be committed? Cheers,
--Aaron On 2020-12-01 08:39 -0600, Aaron Poffenberger <[email protected]> wrote: > Any comments or suggestions? > > On 2020-27-11 10:00 -0600, Aaron Poffenberger <[email protected]> wrote: > > According to rdist(1), the special and cmdspecial commands accept an > > optional list of files. When the <name list> is omitted, the command > > should execute for all files, otherwise it should be executed only > > when one of the listed files is affected. > > > > The special command works as expected, but cmdspecial runs for all > > files regardless of <name list>. > > > > The diff fixes cmdspecial by adding "sc_updfilelist" to the subcmd > > structure to hold a list of affected files found in the <name list> > > (sc_args). For cmdspecial commands without a <name list>, the module > > level variable "updfilelist" is used as before. > > > > The diff also addresses a couple of related issues, which aren't > > technically necessary for the fix, but which helped with debugging and > > testing, and make rdist user messages clearer. I can break them out > > as separate patches if necessary. > > > > Changes: > > - Fix <name list> handling in cmdspecial commands > > - Modify user message to indicate whether special or cmdspecial > > commands apply to "any" file or a "list" of files > > - Show cmdspecial commands that would be executed when the "verify" > > option is set (rdist shows all other actions, including special > > commands, when "verify" is enabled) > > > > > > Test Script > > ----------- > > I've included a shell script that demonstrates the <name list> > > problem. It's attached as a diff for ease of use. I'm not suggesting > > it be committed. > > > > The script creates a Distfile with special and cmdspecial commands > > with both a <name list> and no <name list> specified to highlight > > the difference in handling by rdist. > > > > The script runs the base-installed rdist twice, the first time with > > two changed files, the second with just one. In both cases it executes > > a simple sh script to log the env variable executed with the command > > (special or cmdspecial). > > > > If the script finds an executable rdist in /usr/src/usr.bin/rdist/, it > > then runs that version with the same two test cases. > > > > The script then displays a diff of the two logs showing that the > > patched rdist executes according to the documentation, otherwise it > > shows the output of the first execution to show the defective handling > > of cmdspecial. > > > > --Aaron > > > > cvs diff: Diffing . > > Index: client.c > > =================================================================== > > RCS file: /cvs/src/usr.bin/rdist/client.c,v > > retrieving revision 1.37 > > diff -u -p -r1.37 client.c > > --- client.c 28 Jun 2019 13:35:03 -0000 1.37 > > +++ client.c 27 Nov 2020 14:45:16 -0000 > > @@ -66,7 +66,7 @@ struct namelist *updfilelist = NULL; /* > > > > static void runspecial(char *, opt_t, char *, int); > > static void addcmdspecialfile(char *, char *, int); > > -static void freecmdspecialfiles(void); > > +static void freecmdspecialfiles(struct namelist **); > > static struct linkbuf *linkinfo(struct stat *); > > static int sendhardlink(opt_t, struct linkbuf *, char *, int); > > static int sendfile(char *, opt_t, struct stat *, char *, char *, int); > > @@ -172,7 +172,8 @@ runspecial(char *starget, opt_t opts, ch > > continue; > > if (sc->sc_args != NULL && !inlist(sc->sc_args, starget)) > > continue; > > - message(MT_CHANGE, "special \"%s\"", sc->sc_name); > > + message(MT_CHANGE, "special <%s> \"%s\"", > > + sc->sc_args == NULL ? "any" : "list", sc->sc_name); > > if (IS_ON(opts, DO_VERIFY)) > > continue; > > (void) sendcmd(C_SPECIAL, > > @@ -201,11 +202,19 @@ addcmdspecialfile(char *starget, char *r > > > > rfile = remfilename(source, Tdest, target, rname, destdir); > > > > - for (sc = subcmds; sc != NULL && !isokay; sc = sc->sc_next) { > > + for (sc = subcmds; sc != NULL; sc = sc->sc_next) { > > if (sc->sc_type != CMDSPECIAL) > > continue; > > - if (sc->sc_args != NULL && !inlist(sc->sc_args, starget)) > > + if (sc->sc_args == NULL) { > > + isokay = TRUE; > > continue; > > + } else if (!inlist(sc->sc_args, starget)) > > + continue; > > + new = xmalloc(sizeof *new); > > + new->n_name = xstrdup(rfile); > > + new->n_regex = NULL; > > + new->n_next = sc->sc_updfilelist; > > + sc->sc_updfilelist = new; > > isokay = TRUE; > > } > > > > @@ -222,11 +231,11 @@ addcmdspecialfile(char *starget, char *r > > * Free the file list > > */ > > static void > > -freecmdspecialfiles(void) > > +freecmdspecialfiles(struct namelist **list) > > { > > struct namelist *ptr, *save; > > > > - for (ptr = updfilelist; ptr; ) { > > + for (ptr = *list; ptr; ) { > > if (ptr->n_name) (void) free(ptr->n_name); > > save = ptr->n_next; > > (void) free(ptr); > > @@ -235,7 +244,7 @@ freecmdspecialfiles(void) > > else > > ptr = NULL; > > } > > - updfilelist = NULL; > > + *list = NULL; > > } > > > > /* > > @@ -251,11 +260,15 @@ runcmdspecial(struct cmd *cmd, opt_t opt > > for (sc = cmd->c_cmds; sc != NULL; sc = sc->sc_next) { > > if (sc->sc_type != CMDSPECIAL) > > continue; > > - message(MT_CHANGE, "cmdspecial \"%s\"", sc->sc_name); > > + if (sc->sc_args != NULL && sc->sc_updfilelist == NULL) > > + continue; > > + message(MT_CHANGE, "cmdspecial <%s> \"%s\"", > > + sc->sc_args == NULL ? "any" : "list", sc->sc_name); > > if (IS_ON(opts, DO_VERIFY)) > > continue; > > /* Send all the file names */ > > - for (f = updfilelist; f != NULL; f = f->n_next) { > > + f = sc->sc_updfilelist != NULL ? sc->sc_updfilelist : > > updfilelist; > > + for (; f != NULL; f = f->n_next) { > > if (first) { > > (void) sendcmd(C_CMDSPECIAL, NULL); > > if (response() < 0) > > @@ -277,8 +290,9 @@ runcmdspecial(struct cmd *cmd, opt_t opt > > while (response() > 0) > > ; > > first = TRUE; /* Reset in case there are more CMDSPECIAL's */ > > + freecmdspecialfiles(&sc->sc_updfilelist); > > } > > - freecmdspecialfiles(); > > + freecmdspecialfiles(&updfilelist); > > } > > > > /* > > @@ -1050,6 +1064,7 @@ statupdate(int u, char *starget, opt_t o > > "%s: need to change to perm %#04o, owner %s, > > group %s", > > starget, lmode, user, group); > > runspecial(starget, opts, rname, destdir); > > + addcmdspecialfile(starget, rname, destdir); > > } > > else { > > message(MT_CHANGE, "%s: change to perm %#04o, owner %s, > > group %s", > > @@ -1079,6 +1094,7 @@ fullupdate(int u, char *starget, opt_t o > > if (IS_ON(opts, DO_VERIFY)) { > > message(MT_INFO, "%s: need to install", starget); > > runspecial(starget, opts, rname, destdir); > > + addcmdspecialfile(starget, rname, destdir); > > return(1); > > } > > if (!IS_ON(opts, DO_QUIET)) > > @@ -1110,6 +1126,7 @@ fullupdate(int u, char *starget, opt_t o > > if (IS_ON(opts, DO_VERIFY)) { > > message(MT_INFO, "%s: need to update", starget); > > runspecial(starget, opts, rname, destdir); > > + addcmdspecialfile(starget, rname, destdir); > > return(1); > > } > > if (!IS_ON(opts, DO_QUIET)) > > Index: client.h > > =================================================================== > > RCS file: /cvs/src/usr.bin/rdist/client.h,v > > retrieving revision 1.3 > > diff -u -p -r1.3 client.h > > --- client.h 30 Aug 2017 07:42:52 -0000 1.3 > > +++ client.h 26 Nov 2020 18:14:08 -0000 > > @@ -84,6 +84,7 @@ struct subcmd { > > opt_t sc_options; > > char *sc_name; > > struct namelist *sc_args; > > + struct namelist *sc_updfilelist; > > struct subcmd *sc_next; > > }; > > > > Index: gram.y > > =================================================================== > > RCS file: /cvs/src/usr.bin/rdist/gram.y,v > > retrieving revision 1.12 > > diff -u -p -r1.12 gram.y > > --- gram.y 20 Jan 2015 09:00:16 -0000 1.12 > > +++ gram.y 27 Nov 2020 15:21:31 -0000 > > @@ -592,6 +592,7 @@ makesubcmd(int type) > > sc->sc_type = type; > > sc->sc_args = NULL; > > sc->sc_next = NULL; > > + sc->sc_updfilelist = NULL; > > sc->sc_name = NULL; > > > > return(sc); > > Index: test_cmdspecial > > =================================================================== > > RCS file: test_cmdspecial > > diff -N test_cmdspecial > > --- /dev/null 1 Jan 1970 00:00:00 -0000 > > +++ test_cmdspecial 27 Nov 2020 15:44:43 -0000 > > @@ -0,0 +1,128 @@ > > +#!/bin/sh > > + > > +TEST_DIR=/tmp/rdist > > +rm -Rf $TEST_DIR* > > +mkdir -p "$TEST_DIR/src" "$TEST_DIR/tgt" > > + > > +if [ -x ./rdist ]; then > > + rdist_p="$(pwd)/rdist" > > +elif [ -x /usr/src/usr.bin/rdist/rdist ]; then > > + rdist_p="/usr/src/usr.bin/rdist/rdist" > > +fi > > + > > +cd "$TEST_DIR" > > + > > +# Create Distfile > > +cat <<- EOF > Distfile > > +test: > > +src -> localhost > > + install tgt ; > > + special src/file "echo special src/file env_REMFILE=\$REMFILE >> \ > > +rdist.log" ; > > + special "echo special any env_REMFILE=\$REMFILE >> rdist.log" ; > > + cmdspecial src/file "echo cmdspecial src/file env_FILES=\$FILES >> \ > > +rdist.log" ; > > + cmdspecial "echo cmdspecial any env_FILES=\$FILES >> rdist.log" ; > > +EOF > > + > > +# Run unpatched rdist with both files modified > > +# Both special and cmdspecial commands will execute > > +# NOTE: the FILES variable for cmdspecial *with* a <name list> > > +# will include the names of both files when it should > > +# only include the affected files in the <name list> > > + > > +echo "" > > +#echo "Unpatched rdist - update both files:" >> rdist.log > > +touch -d 2020-01-01T08:00:00 src/file{,2} > > +/usr/bin/rdist > /dev/null > > +mv rdist{,_orgnl_both_files}.log > > + > > + > > +# Run unpatched rdist with only file2 modified > > +# The special command with no <name list> will execute as it should > > +# The special command *with* a <name list> will not > > +# NOTE: Both cmdspecial commands will execute > > +# The cmdspecial command *with* a <name list> should > > +# not execute! > > + > > +#echo "Unpatched rdist - update file2 only:" >> rdist.log > > +touch -d 2020-01-01T09:00:00 src/file2 > > +/usr/bin/rdist > /dev/null > > +mv rdist{,_orgnl_file2_only}.log > > + > > + > > +if [ -n "$rdist_p" ]; then > > + rm src/* tgt/* > > + > > + # Run unpatched rdist with both files modified > > + # Both special and cmdspecial commands will execute > > + # NOTE: the FILES variable for the cmdspecial *with* a > > + # <name list> will include only the specified file! > > + > > + touch -d 2020-01-01T08:00:00 src/file{,2} > > + "$rdist_p" > /dev/null > > + mv rdist{,_patch_both_files}.log > > + > > + > > + # Run unpatched rdist with only file2 modified > > + # The special command with no <name list> executes as expected > > + # The special command *with* a <name list> will not > > + # NOTE: Only the cmdspecial command *without* a <name list> > > + # will execute, as it should > > + > > + touch -d 2020-01-01T09:00:00 src/file2 > > + "$rdist_p" > /dev/null > > + mv rdist{,_patch_file2_only}.log > > + > > + cat <<- EOF > > + Patched rdist: $rdist_p > > + > > + Diff logs when both files are affected > > + -------------------------------------- > > + rdist_orgnl_both_files.log: > > + - The FILES variable for cmdspecial *with* > > + a specified <name list> shows both files! > > + rdist_patch_both_files.log: > > + - FILES shows only the expected file. > > + > > + EOF > > + diff -u "$TEST_DIR/rdist_orgnl_both_files.log" \ > > + "$TEST_DIR/rdist_patch_both_files.log" > > + > > + cat <<- EOF > > + > > + Diff logs when only one file is affected > > + ---------------------------------------- > > + rdist_orgnl_file2_only.log: > > + - The FILES variable for the cmdspecial *with* > > + "src/file" in the <name list> still runs! > > + rdist_patch_file2_only.log: > > + - The cmdspecial doesn't run, as expected. > > + > > + EOF > > + diff -u "$TEST_DIR/rdist_orgnl_file2_only.log" \ > > + "$TEST_DIR/rdist_patch_file2_only.log" > > +else > > + cat <<- EOF > > + Patched rdist not found > > + > > + Log when both files are affected > > + -------------------------------------- > > + rdist_orgnl_both_files.log: > > + - The FILES variable for cmdspecial *with* > > + a specified <name list> shows both files! > > + > > + EOF > > + cat rdist_orgnl_both_files.log > > + > > + cat <<- EOF > > + > > + Log when only one file is affected > > + ---------------------------------------- > > + rdist_orgnl_file2_only.log: > > + - The FILES variable for the cmdspecial *with* > > + "src/file" in the <name list> still runs! > > + > > + EOF > > + cat rdist_orgnl_file2_only.log > > +fi > > >
