adb: make pulling symlinks and devices work.

Bug: http://b/25972989
Bug: http://b/26085751
Change-Id: I43842871522ea5f67a8c258dcb6ddafa8dd744c8
diff --git a/file_sync_client.cpp b/file_sync_client.cpp
index 463c1c0..c7eeea7 100644
--- a/file_sync_client.cpp
+++ b/file_sync_client.cpp
@@ -503,16 +503,6 @@
     });
 }
 
-struct copyinfo
-{
-    std::string lpath;
-    std::string rpath;
-    unsigned int time;
-    unsigned int mode;
-    uint64_t size;
-    bool skip;
-};
-
 static void ensure_trailing_separator(std::string& lpath, std::string& rpath) {
     if (!adb_is_separator(lpath.back())) {
         lpath.push_back(OS_PATH_SEPARATOR);
@@ -522,25 +512,26 @@
     }
 }
 
-static copyinfo mkcopyinfo(std::string lpath, std::string rpath,
-                           const std::string& name, unsigned int mode) {
-    copyinfo result;
-    result.lpath = std::move(lpath);
-    result.rpath = std::move(rpath);
-    ensure_trailing_separator(result.lpath, result.rpath);
-    result.lpath.append(name);
-    result.rpath.append(name);
+struct copyinfo {
+    std::string lpath;
+    std::string rpath;
+    unsigned int time = 0;
+    unsigned int mode;
+    uint64_t size = 0;
+    bool skip = false;
 
-    if (S_ISDIR(mode)) {
-        ensure_trailing_separator(result.lpath, result.rpath);
+    copyinfo(const std::string& lpath, const std::string& rpath, const std::string& name,
+             unsigned int mode)
+        : lpath(lpath), rpath(rpath), mode(mode) {
+        ensure_trailing_separator(this->lpath, this->rpath);
+        this->lpath.append(name);
+        this->rpath.append(name);
+
+        if (S_ISDIR(mode)) {
+            ensure_trailing_separator(this->lpath, this->rpath);
+        }
     }
-
-    result.time = 0;
-    result.mode = mode;
-    result.size = 0;
-    result.skip = false;
-    return result;
-}
+};
 
 static bool IsDotOrDotDot(const char* name) {
     return name[0] == '.' && (name[1] == '\0' || (name[1] == '.' && name[2] == '\0'));
@@ -573,7 +564,7 @@
             continue;
         }
 
-        copyinfo ci = mkcopyinfo(lpath, rpath, de->d_name, st.st_mode);
+        copyinfo ci(lpath, rpath, de->d_name, st.st_mode);
         if (S_ISDIR(st.st_mode)) {
             dirlist.push_back(ci);
         } else {
@@ -597,8 +588,7 @@
         // TODO(b/25566053): Make pushing empty directories work.
         // TODO(b/25457350): We don't preserve permissions on directories.
         sc.Warning("skipping empty directory '%s'", lpath.c_str());
-        copyinfo ci = mkcopyinfo(adb_dirname(lpath), adb_dirname(rpath),
-                                 adb_basename(lpath), S_IFDIR);
+        copyinfo ci(adb_dirname(lpath), adb_dirname(rpath), adb_basename(lpath), S_IFDIR);
         ci.skip = true;
         filelist->push_back(ci);
         return true;
@@ -743,11 +733,23 @@
     return success;
 }
 
+static bool remote_symlink_isdir(SyncConnection& sc, const std::string& rpath) {
+    unsigned mode;
+    std::string dir_path = rpath;
+    dir_path.push_back('/');
+    if (!sync_stat(sc, dir_path.c_str(), nullptr, &mode, nullptr)) {
+        sc.Error("failed to stat remote symlink '%s'", dir_path.c_str());
+        return false;
+    }
+    return S_ISDIR(mode);
+}
+
 static bool remote_build_list(SyncConnection& sc,
                               std::vector<copyinfo>* filelist,
                               const std::string& rpath,
                               const std::string& lpath) {
     std::vector<copyinfo> dirlist;
+    std::vector<copyinfo> linklist;
     bool empty_dir = true;
 
     // Put the files/dirs in rpath on the lists.
@@ -759,20 +761,14 @@
         // We found a child that isn't '.' or '..'.
         empty_dir = false;
 
-        copyinfo ci = mkcopyinfo(lpath, rpath, name, mode);
+        copyinfo ci(lpath, rpath, name, mode);
         if (S_ISDIR(mode)) {
             dirlist.push_back(ci);
+        } else if (S_ISLNK(mode)) {
+            linklist.push_back(ci);
         } else {
-            if (S_ISREG(mode)) {
-                ci.time = time;
-                ci.size = size;
-            } else if (S_ISLNK(mode)) {
-                sc.Warning("skipping symlink '%s'", name);
-                ci.skip = true;
-            } else {
-                sc.Warning("skipping special file '%s'", name);
-                ci.skip = true;
-            }
+            ci.time = time;
+            ci.size = size;
             filelist->push_back(ci);
         }
     };
@@ -781,14 +777,22 @@
         return false;
     }
 
-    // Add the current directory to the list if it was empty, to ensure that
-    // it gets created.
+    // Add the current directory to the list if it was empty, to ensure that it gets created.
     if (empty_dir) {
-        filelist->push_back(mkcopyinfo(adb_dirname(lpath), adb_dirname(rpath),
-                                       adb_basename(rpath), S_IFDIR));
+        copyinfo ci(adb_dirname(lpath), adb_dirname(rpath), adb_basename(rpath), S_IFDIR);
+        filelist->push_back(ci);
         return true;
     }
 
+    // Check each symlink we found to see whether it's a file or directory.
+    for (copyinfo& link_ci : linklist) {
+        if (remote_symlink_isdir(sc, link_ci.rpath)) {
+            dirlist.emplace_back(std::move(link_ci));
+        } else {
+            filelist->emplace_back(std::move(link_ci));
+        }
+    }
+
     // Recurse into each directory we found.
     while (!dirlist.empty()) {
         copyinfo current = dirlist.back();
@@ -912,6 +916,7 @@
         const char* dst_path = dst;
         unsigned src_mode, src_time;
         if (!sync_stat(sc, src_path, &src_time, &src_mode, nullptr)) {
+            sc.Error("failed to stat remote object '%s'", src_path);
             return false;
         }
         if (src_mode == 0) {
@@ -920,28 +925,17 @@
             continue;
         }
 
-        if (S_ISREG(src_mode)) {
-            std::string path_holder;
-            if (dst_isdir) {
-                // If we're copying a remote file to a local directory, we
-                // really want to copy to local_dir + OS_PATH_SEPARATOR +
-                // basename(remote).
-                path_holder = android::base::StringPrintf(
-                    "%s%c%s", dst_path, OS_PATH_SEPARATOR,
-                    adb_basename(src_path).c_str());
-                dst_path = path_holder.c_str();
-            }
-            if (!sync_recv(sc, src_path, dst_path)) {
-                success = false;
-                continue;
-            } else {
-                if (copy_attrs &&
-                    set_time_and_mode(dst_path, src_time, src_mode) != 0) {
-                    success = false;
-                    continue;
-                }
-            }
-        } else if (S_ISDIR(src_mode)) {
+        bool src_isdir = S_ISDIR(src_mode);
+        if (S_ISLNK(src_mode)) {
+            src_isdir = remote_symlink_isdir(sc, src_path);
+        }
+
+        if ((src_mode & (S_IFREG | S_IFDIR | S_IFBLK | S_IFCHR)) == 0) {
+            sc.Error("skipping remote object '%s' (mode = 0o%o)", src_path, src_mode);
+            continue;
+        }
+
+        if (src_isdir) {
             std::string dst_dir = dst;
 
             // If the destination path existed originally, the source directory
@@ -957,13 +951,28 @@
                 dst_dir.append(adb_basename(src_path));
             }
 
-            success &= copy_remote_dir_local(sc, src_path, dst_dir.c_str(),
-                                             copy_attrs);
+            success &= copy_remote_dir_local(sc, src_path, dst_dir.c_str(), copy_attrs);
             continue;
         } else {
-            sc.Error("remote object '%s' not a file or directory", src_path);
-            success = false;
-            continue;
+            std::string path_holder;
+            if (dst_isdir) {
+                // If we're copying a remote file to a local directory, we
+                // really want to copy to local_dir + OS_PATH_SEPARATOR +
+                // basename(remote).
+                path_holder = android::base::StringPrintf("%s%c%s", dst_path, OS_PATH_SEPARATOR,
+                                                          adb_basename(src_path).c_str());
+                dst_path = path_holder.c_str();
+            }
+
+            if (!sync_recv(sc, src_path, dst_path)) {
+                success = false;
+                continue;
+            }
+
+            if (copy_attrs && set_time_and_mode(dst_path, src_time, src_mode) != 0) {
+                success = false;
+                continue;
+            }
         }
     }
 
diff --git a/test_device.py b/test_device.py
index 955b67a..afc061a 100644
--- a/test_device.py
+++ b/test_device.py
@@ -829,6 +829,40 @@
             if host_dir is not None:
                 shutil.rmtree(host_dir)
 
+    def test_pull_symlink_dir(self):
+        """Pull a symlink to a directory of symlinks to files."""
+        try:
+            host_dir = tempfile.mkdtemp()
+
+            remote_dir = posixpath.join(self.DEVICE_TEMP_DIR, 'contents')
+            remote_links = posixpath.join(self.DEVICE_TEMP_DIR, 'links')
+            remote_symlink = posixpath.join(self.DEVICE_TEMP_DIR, 'symlink')
+
+            self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
+            self.device.shell(['mkdir', '-p', remote_dir, remote_links])
+            self.device.shell(['ln', '-s', remote_links, remote_symlink])
+
+            # Populate device directory with random files.
+            temp_files = make_random_device_files(
+                self.device, in_dir=remote_dir, num_files=32)
+
+            for temp_file in temp_files:
+                self.device.shell(
+                    ['ln', '-s', '../contents/{}'.format(temp_file.base_name),
+                     posixpath.join(remote_links, temp_file.base_name)])
+
+            self.device.pull(remote=remote_symlink, local=host_dir)
+
+            for temp_file in temp_files:
+                host_path = os.path.join(
+                    host_dir, 'symlink', temp_file.base_name)
+                self._verify_local(temp_file.checksum, host_path)
+
+            self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
+        finally:
+            if host_dir is not None:
+                shutil.rmtree(host_dir)
+
     def test_pull_empty(self):
         """Pull a directory containing an empty directory from the device."""
         try: