Bug-871: mv builtins cannot move on cross-mountpoint, cp obscure error

src/cmd/ksh93/tests/libcmd.sh

@@ -950,5 +950,92 @@ if builtin join 2>/dev/null; then
 		"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 fi
 
+# ======
+# https://github.com/ksh93/ksh/issues/871
+# The mv test requires 2 different mounted FSs; since $tmp is located in /tmp
+# we will create yet another tmp located in the artefact dir $INSTALLROOT
+builtin cp 2>/dev/null &&
+builtin mv 2>/dev/null &&
+{
+	T2=$INSTALLROOT/src/lib/libcmd/tmp
+	rm -rf "$T2" 
+	mkdir "$T2" || exit
+
+	rm -rf a b t u v w
+	mkdir t v || exit
+	echo T>t/T U>u V>v/V W>w
+
+	got=$(cp t x 2>&1)
+	exp="cp: warning: -r not specified; omitting directory t"
+	[ "$got" = "$exp" ] || err_exit "builtin cp: exp $(printf %q "$exp") got $(printf %q "$got")"
+
+	cp -r [tuvw] "$T2"
+	got=$( find "$T2"/[tuvw] | sed "s:$T2/::")
+	exp=$(find [tuvw])
+	[ "$got" = "$exp" ] || err_exit "builtin cp: exp $(printf %q "$exp") got $(printf %q "$got")"
+	rm -rf "$T2"/[tuvw]
+
+# Bogus builtin error message on nonexistent target path
+# Even the -h doesn't work as specified (create target dirs)
+# So here we expect the bogus error message in lieu of
+# mv: target '/tmp/d1/d2': No such file or directory
+	exp1="$exp" 
+	got=$(mv [tuvw] "$T2"/d1/d2 2>&1)
+	exp='Usage: '
+	# [[ $got == "$exp"* ]] || err_exit "builtin mv: expected match of $(printf %q "$exp")*, got $(printf %q "$got")"
+
+	# Check the above expected failure didn't destroy [tuvw]
+	got=$(find [tuvw])
+	exp="$exp1"
+	[[ $got == "$exp" ]] || err_exit "builtin mv: src destoyed: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# missing file along the args, should cause failure and no src... destruction.
+	rm -rf z
+	mkdir z || exit
+	got=$(mv t a u u/m v w z 2>&1)
+	exp=\
+'mv: a: not found
+mv: u/m: not found
+mv: warning: src not removed due to errors'
+	[[ $got == "$exp" ]] || err_exit "builtin mv: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# Check the above expected failure didn't destroy [tuvw]
+	got=$(find [tuvw])
+	exp="$exp1"
+	[[ $got == "$exp" ]] || err_exit "builtin mv: src destoyed: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# mv files and dirs into the same mount point.
+	rm -rf z
+	mkdir z || exit
+	got=$(mv t u v w z 2>&1)
+	exp=''
+	[[ $got == "$exp" ]] || err_exit "builtin mv: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# mv files back same mount point.
+	got=$(mv z/* . 2>&1 ; find [tuvw])
+	exp="$exp1"
+ 	[[ $got == "$exp" ]] || err_exit "builtin mv: restore failed: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# mv files and dirs into the xdev mount point.
+	rm -rf "$T2/z"
+	mkdir "$T2/z" || exit
+	got=$(mv t u v w "$T2"/z 2>&1)
+	exp=''
+	[[ $got == "$exp" ]] || err_exit "builtin mv xdev: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# Check src's removed.
+	got=$(find [tuvw] 2>/dev/null)
+	exp=''
+	[[ $got == "$exp" ]] || err_exit "builtin mv xdev, src's left over: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+# Check dst
+	got=$(find "$T2"/z/[tuvw] 2>&1 | sed "s:"$T2"/z/::" )
+	exp="$exp1"
+	[[ $got == "$exp" ]] || err_exit "builtin mv xdev, dst wrong: expected $(printf %q "$exp"), got $(printf %q "$got")"
+
+	rm -rf "$T2"
+	trap - EXIT
+}
+
 # ======
 exit $((Errors<125?Errors:125))

src/lib/libcmd/cp.c

@@ -679,8 +679,31 @@ b_cp(int argc, char** argv, Shbltin_t* context)
 	State_t*	state;
 	Shbltin_t*	sh;
 	Shbltin_t*	cleanup = context;
+	char*		cmd;   /* bug-871: Phi: */
+	int		xmv=0; /* bug-871: Phi: */
 
+	/* bug-871: Phi: b_mv() want cp -r on xdev*/
+	if(argv[0][0]=='X')
+	{
+	  argv[0]="cp";
+	  xmv=1;
+	}
 	cmdinit(argc, argv, context, ERROR_CATALOG, ERROR_NOTIFY);
+
+	/*
+	 * bug-871: Phi:
+	 * The above cmdinit() do set error_info.id to cp|mv|ln from argv[0]
+	 * If we are called from b_mv() in the xdev case, we keep the
+	 * current error_id ("cp") int cmd, then we restore the error_info.id
+	 * to "mv" for accurate error message reporting.
+	 * The backup'ed error_info.id in cmd is then used down.
+	 * b_mv() in the non xdev (same FS) call b_cp() with argv[0]="mv" in
+	 * which case we don't enter the new code (cp -r; rm )
+	 */
+	cmd=error_info.id;
+	if(xmv)
+		error_info.id="mv";
+
 	if (!(sh = CMD_CONTEXT(context)) || !(state = (State_t*)sh->ptr))
 	{
 		if (!(state = newof(0, State_t, 1, 0)))
@@ -706,7 +729,7 @@ b_cp(int argc, char** argv, Shbltin_t* context)
 	}
 	sfputr(state->tmp, usage_head, -1);
 	standard = !!conformance(0, 0);
-	switch (error_info.id[0])
+	switch (cmd[0]) /* bug-871: Phi: */
 	{
 	case 'c':
 	case 'C':
@@ -985,7 +1008,26 @@ b_cp(int argc, char** argv, Shbltin_t* context)
 		state->flags |= FTS_TOP;
 	if (fts = fts_open(argv, state->flags, NULL))
 	{
-		while (!sh_checksig(context) && (ent = fts_read(fts)) && !visit(state, ent));
+		/* bug-871: Phi: */
+		while (!sh_checksig(context) && (ent = fts_read(fts)))
+		{
+			/*
+			 * bug-871: Phi:
+			 * If src is a dir and -r was not given, skip src
+			 */
+			if( cmd[0]=='c' &&
+			    S_ISDIR(ent->fts_statp->st_mode) &&
+			    !state->recursive )
+			{
+				error(1,
+					"-r not specified; omitting directory %s",
+					ent->fts_path);
+				fts_read(fts);
+				continue;
+			}
+			if(visit(state, ent))
+				break;
+		}
 		fts_close(fts);
 	}
 	else if (state->link != pathsetlink)

src/lib/libcmd/mv.c

@@ -27,5 +27,70 @@
 int
 b_mv(int argc, char** argv, Shbltin_t* context)
 {
+	/*
+	 * bug-871: Phi: Catch xdev mv
+	 * If any of the src is not on same dev as the dst one it is an
+	 * xdev move and b_cp() in MV mode don't handle it correctly.
+	 * So the gross hack here is a work around, on xdev move
+	 * we do a cp -r src... dst (which seems to work)
+	 * then on success we do a rm -r src...
+	 * On degenerate case, i.e case where we can not figure out the dst dev
+         * we go the xdev path (cp;rm) and let cp deal with errors.
+	 */
+	int		ac;
+	char		**av;
+	struct stat	sts, std;
+	int		i;	  
+	int		xdev=0;
+
+	if(argc>2)
+	{
+		if(stat(argv[argc-1], &std))
+		{ std.st_dev=0;			
+		}
+		for(i=1;i<argc-1;i++)
+		{ 
+			if(stat(argv[i], &sts))
+				sts.st_dev=0;
+
+			if(sts.st_dev!=std.st_dev)
+			     xdev++;
+		}
+		if(xdev)
+		{
+			ac=argc+1;
+			av=malloc( (ac+1)*sizeof(char*) );
+			if(av) /* Let fallthrough report av==0 */
+			{
+				av[0]="XM"; /* Our sig for b_cp() */
+				av[1]="-r";
+				for(i=1;i<=argc;i++)
+				{ av[1+i]=argv[i];
+				}
+				i=b_cp(ac,av,context);
+				if(i)
+				{
+					error(1|ERROR_WARNING,
+					      "src not removed due to errors");
+
+					free(av);
+					return(i);
+				}
+				av[0]="rm";
+                                av[1]="-rfd";
+                                ac--;
+                                av[ac]=0;
+				for(i=2;i<ac;i++)
+				{ if(av[i][0]=='-')
+                                  { av[i]="-r";
+                                  }
+				}
+                                i=b_rm(ac,av,context);
+				free(av);
+				return(i);
+			}
+		}
+	}
+
 	return b_cp(argc, argv, context);
 }