rsync - Aug 2004 - Using --keep-dirlinks : recursive symlinks problem

Hello folks,

trying to make use of the new --keep-dirlinks feature for a synch port; 
it works excellent but feels incomplete without a way for rsync to 
ignore recursive links.

Example:
# ls -la .
total 6
drwxrwxr-x   2 bldmstr  staff       512 Aug 16 21:42 .
drwxrwxr-x   8 bldmstr  staff       512 Aug 16 21:41 ..
lrwxrwxrwx   1 bldmstr  staff         2 Aug 20 12:07 bogus -> ..

if you use '--keep-dirlinks --delete' rsync goes into recursion trying 
to delete unexisting directory at destination.

Is it possible to introduce a feature, or make it a default to ignore 
recursive dirlinks under --keep-dirlinks? Highly doubt this behaviour 
would ever be called for :)

Or please kick me in the right direction for a workaround which would 
make --keep-dirlinks consider sane symlinks only.

thanks for your time,
-- 
Ivan S. Manida, cdev/buildmaster
Sun Microsystems

On Fri, Aug 20, 2004 at 04:50:45PM +0400, Ivan S. Manida
wrote:
> lrwxrwxrwx   1 bldmstr  staff         2 Aug 20 12:07 bogus -> ..
> 
> if you use '--keep-dirlinks --delete' rsync goes into recursion
trying
> to delete unexisting directory at destination.
Yeah, that's what all older rsyncs would do when --copy-links was used
in that situation (--copy-links used to have the unexpected side-effect
on the receiver of the new --keep-dirlinks option).  I also note that a
looping symlink on the sender would result in a similar loop trying to
find all the files to send (it's not infinite, since the names will
eventually either exceed the symlink recursion limit or the maximum
path-length).

I'm debating whether this is something that should be guarded against or
not.  Opinions welcomed.

..wayne..

It is not infinite, but it is inconvenient since rsync would generate a transfer
error. I think it makes sense to detect such symlinks and have an option which
would modify copy-links and keep-dirlinks behavior, since it's not always
possible to modify the sending filesystem to correct the loop.


----- Original Message -----
From: Wayne Davison
Date: Sunday, August 22, 2004 8:05 am
> Yeah, that's what all older rsyncs would do when --copy-links was used
> in that situation (--copy-links used to have the unexpected side-
> effecton the receiver of the new --keep-dirlinks option).  I also 
> note that a
> looping symlink on the sender would result in a similar loop 
> trying to
> find all the files to send (it's not infinite, since the names will
> eventually either exceed the symlink recursion limit or the maximum
> path-length).
> 
> I'm debating whether this is something that should be guarded 
> against or
> not.  Opinions welcomed.
>

On Fri, Aug 20, 2004 at 04:50:45PM +0400, Ivan S. Manida
wrote:
> Or please kick me in the right direction for a workaround which would 
> make --keep-dirlinks consider sane symlinks only.
Seems like the only good solution for this is to keep track of the
device and inode of all the dirs we visit so that we can eliminate all
duplicate directories.  Attached is a patch that does this using a
simple binary insertion sort.  Very minimally tested.  Thoughts?
Optimizations?

..wayne..
-------------- next part --------------
--- flist.c	12 Aug 2004 18:20:07 -0000	1.236
+++ flist.c	25 Aug 2004 07:27:12 -0000
@@ -724,6 +724,43 @@ void receive_file_entry(struct file_stru
 }
 
 
+static BOOL saw_dir(dev_t dev, ino_t ino)
+{
+	static struct dirinfo { dev_t dev; ino_t ino; } *dirarray;
+	static int dirarray_cnt, dirarray_size;
+	int low, high;
+
+	if (dirarray_cnt == dirarray_size) {
+		dirarray = realloc_array(dirarray, struct dirinfo,
+					 dirarray_size += 4096);
+	}
+
+	for (low = 0, high = dirarray_cnt - 1; low <= high; ) {
+		int j = (low + high) / 2;
+		if (ino == dirarray[j].ino) {
+			if (dev == dirarray[j].dev)
+				return True;
+			if (dev > dirarray[j].dev)
+				low = j + 1;
+			else
+				high = j - 1;
+		} else if (ino > dirarray[j].ino)
+			low = j + 1;
+		else
+			high = j - 1;
+	}
+
+	if (low < dirarray_cnt) {
+		memmove(dirarray + low + 1, dirarray + low,
+			(dirarray_cnt - low) * sizeof dirarray[0]);
+	}
+	dirarray[low].dev = dev;
+	dirarray[low].ino = ino;
+	dirarray_cnt++;
+
+	return False;
+}
+
 /**
  * Create a file_struct for a named file by reading its stat()
  * information and performing extensive checks against global
@@ -802,9 +839,14 @@ struct file_struct *make_file(char *fnam
 	if (exclude_level == NO_EXCLUDES)
 		goto skip_excludes;
 
-	if (S_ISDIR(st.st_mode) && !recurse && !files_from) {
-		rprintf(FINFO, "skipping directory %s\n", thisname);
-		return NULL;
+	if (S_ISDIR(st.st_mode)) {
+		if (!recurse && !files_from) {
+			rprintf(FINFO, "skipping directory %s\n", thisname);
+			return NULL;
+		}
+		if ((keep_dirlinks || copy_links)
+		    && saw_dir(st.st_dev, st.st_ino))
+			return NULL;
 	}
 
 	/* We only care about directories because we need to avoid recursing