Bug 1316735 - Relative symlinks in the dist directory. r?gps draft
authorJack Bates <jack@nottheoilrig.com>
Thu, 10 Nov 2016 22:24:38 +0000
changeset 442592 38517c2bd352fab3e377c42ae42d4d9c8262725a
parent 429415 c1eb8c89f5d9094564f5eeb6d618dae0e7395b4d
child 537832 3781e1b110b1349418b1a8a8d026925fef6f216e
push id36745
push userbmo:jack@nottheoilrig.com
push dateTue, 22 Nov 2016 20:53:16 +0000
reviewersgps
bugs1316735
milestone52.0a1
Bug 1316735 - Relative symlinks in the dist directory. r?gps Make the symlinks in the dist directory relative instead of absolute. MozReview-Commit-ID: HS7KL4JwSbV
config/nsinstall.c
python/mozbuild/mozbuild/jar.py
python/mozbuild/mozbuild/test/test_jarmaker.py
python/mozbuild/mozpack/files.py
python/mozbuild/mozpack/test/test_files.py
--- a/config/nsinstall.c
+++ b/config/nsinstall.c
@@ -258,23 +258,23 @@ copydir( char *from, char *to, mode_t mo
   closedir(dir);
 }
 
 int
 main(int argc, char **argv)
 {
     int onlydir, dodir, dolink, dorelsymlink, dotimes, opt, len, lplen, tdlen, bnlen, exists;
     mode_t mode = 0755;
-    char *linkprefix, *owner, *group, *cp, *cwd, *todir, *toname, *name, *base, *linkname, buf[BUFSIZ];
+    char *linkprefix, *owner, *group, *cp, *cwd, *todir, *toname, *name, *base, *linkname, *absolute, buf[BUFSIZ];
     uid_t uid;
     gid_t gid;
     struct stat sb, tosb, fromsb;
 
     program = argv[0];
-    cwd = linkname = linkprefix = owner = group = 0;
+    cwd = linkname = absolute = linkprefix = owner = group = 0;
     onlydir = dodir = dolink = dorelsymlink = dotimes = lplen = 0;
 
     while ((opt = getopt(argc, argv, "C:DdlL:Rm:o:g:t")) != EOF) {
 	switch (opt) {
 	  case 'C':
 	    cwd = optarg;
 	    break;
 	  case 'D':
@@ -374,43 +374,43 @@ main(int argc, char **argv)
 	    if (!exists && mkdir(toname, mode) < 0)
 		fail("cannot make directory %s", toname);
 	    if ((owner || group) && chown(toname, uid, gid) < 0)
 		fail("cannot change owner of %s", toname);
 	} else if (dolink) {
             if (access(name, R_OK) != 0) {
                 fail("cannot access %s", name);
             }
-	    if (*name == '/') {
-		/* source is absolute pathname, link to it directly */
-		linkname = 0;
-	    } else {
-		if (linkprefix) {
-		    /* -L prefixes names with a $cwd arg. */
-		    len += lplen + 1;
-		    linkname = xmalloc((unsigned int)(len + 1));
-		    sprintf(linkname, "%s/%s", linkprefix, name);
-		} else if (dorelsymlink) {
-		    /* Symlink the relative path from todir to source name. */
-		    linkname = xmalloc(PATH_MAX);
+	    if (linkprefix) {
+		/* -L prefixes names with a $cwd arg. */
+		len += lplen + 1;
+		linkname = xmalloc((unsigned int)(len + 1));
+		sprintf(linkname, "%s/%s", linkprefix, name);
+	    } else if (dorelsymlink) {
+		if (*name != '/') {
+		    absolute = xmalloc((unsigned int)(strlen(cwd) + 1 + len + 1));
+		    sprintf(absolute, "%s/%s", cwd, name);
+		    name = absolute;
+		}
 
-		    if (*todir == '/') {
-			/* todir is absolute: skip over common prefix. */
-			lplen = relatepaths(todir, cwd, linkname);
-			strcpy(linkname + lplen, name);
-		    } else {
-			/* todir is named by a relative path: reverse it. */
-			reversepath(todir, name, len, linkname);
-			xchdir(cwd);
-		    }
+		/* Symlink the relative path from todir to source name. */
+		linkname = xmalloc(PATH_MAX);
 
-		    len = strlen(linkname);
+		/* todir is absolute: skip over common prefix. */
+		len = relatepaths(todir, name, linkname);
+		if (len > 0) {
+		    linkname[--len] = '\0';
 		}
-		name = linkname;
+
+		if (absolute) {
+		    free(absolute);
+		    absolute = 0;
+		}
 	    }
+	    name = linkname;
 
 	    /* Check for a pre-existing symlink with identical content. */
 	    if (exists && (!S_ISLNK(tosb.st_mode) ||
 						readlink(toname, buf, sizeof buf) != len ||
 						strncmp(buf, name, (unsigned int)len) != 0 || 
 			((stat(name, &fromsb) == 0) &&
 			 (fromsb.st_mtime > tosb.st_mtime) 
 			 ))) {
--- a/python/mozbuild/mozbuild/jar.py
+++ b/python/mozbuild/mozbuild/jar.py
@@ -540,16 +540,17 @@ class JarMaker(object):
 
             # remove previous link or file
             try:
                 os.remove(out)
             except OSError, e:
                 if e.errno != errno.ENOENT:
                     raise
             if sys.platform != 'win32':
+                src = os.path.relpath(src, os.path.dirname(out))
                 os.symlink(src, out)
             else:
                 # On Win32, use ctypes to create a hardlink
                 rv = CreateHardLink(out, src, None)
                 if rv == 0:
                     raise WinError()
 
 
--- a/python/mozbuild/mozbuild/test/test_jarmaker.py
+++ b/python/mozbuild/mozbuild/test/test_jarmaker.py
@@ -104,17 +104,19 @@ def is_symlink_to(dest, src):
         srcinfo = _getfileinfo(src)
         return (destinfo.dwVolumeSerialNumber == srcinfo.dwVolumeSerialNumber and
                 destinfo.nFileIndexHigh == srcinfo.nFileIndexHigh and
                 destinfo.nFileIndexLow == srcinfo.nFileIndexLow)
     else:
         # Read the link and check if it is correct
         if not os.path.islink(dest):
             return False
-        target = os.path.abspath(os.readlink(dest))
+        target = os.readlink(dest)
+        target = os.path.join(os.path.dirname(dest), target)
+        target = os.path.abspath(target)
         abssrc = os.path.abspath(src)
         return target == abssrc
 
 class _TreeDiff(dircmp):
     """Helper to report rich results on difference between two directories.
     """
     def _fillDiff(self, dc, rv, basepath="{0}"):
         rv['right_only'] += map(lambda l: basepath.format(l), dc.right_only)
--- a/python/mozbuild/mozpack/files.py
+++ b/python/mozbuild/mozpack/files.py
@@ -318,33 +318,35 @@ class AbsoluteSymlinkFile(File):
         st = None
 
         try:
             st = os.lstat(dest)
         except OSError as ose:
             if ose.errno != errno.ENOENT:
                 raise
 
+        src = os.path.relpath(self.path, os.path.dirname(dest))
+
         # If the dest is a symlink pointing to us, we have nothing to do.
         # If it's the wrong symlink, the filesystem must support symlinks,
         # so we replace with a proper symlink.
         if st and stat.S_ISLNK(st.st_mode):
             link = os.readlink(dest)
-            if link == self.path:
+            if link == src:
                 return False
 
             os.remove(dest)
-            os.symlink(self.path, dest)
+            os.symlink(src, dest)
             return True
 
         # If the destination doesn't exist, we try to create a symlink. If that
         # fails, we fall back to copy code.
         if not st:
             try:
-                os.symlink(self.path, dest)
+                os.symlink(src, dest)
                 return True
             except OSError:
                 return File.copy(self, dest, skip_if_older=skip_if_older)
 
         # Now the complicated part. If the destination exists, we could be
         # replacing a file with a symlink. Or, the filesystem may not support
         # symlinks. We want to minimize I/O overhead for performance reasons,
         # so we keep the existing destination file around as long as possible.
@@ -357,17 +359,17 @@ class AbsoluteSymlinkFile(File):
         #
         # Our strategy is to attempt to create a new symlink with a random
         # name. If that fails, we fall back to copy mode. If that works, we
         # remove the old destination and move the newly-created symlink into
         # its place.
 
         temp_dest = os.path.join(os.path.dirname(dest), str(uuid.uuid4()))
         try:
-            os.symlink(self.path, temp_dest)
+            os.symlink(src, temp_dest)
         # TODO Figure out exactly how symlink creation fails and only trap
         # that.
         except EnvironmentError:
             return File.copy(self, dest, skip_if_older=skip_if_older)
 
         # If removing the original file fails, don't forget to clean up the
         # temporary symlink.
         try:
--- a/python/mozbuild/mozpack/test/test_files.py
+++ b/python/mozbuild/mozpack/test/test_files.py
@@ -278,17 +278,17 @@ class TestAbsoluteSymlinkFile(TestWithTm
 
         s = AbsoluteSymlinkFile(source)
         dest = self.tmppath('symlink')
         self.assertTrue(s.copy(dest))
 
         if self.symlink_supported:
             self.assertTrue(os.path.islink(dest))
             link = os.readlink(dest)
-            self.assertEqual(link, source)
+            self.assertEqual(link, 'test_path')
         else:
             self.assertTrue(os.path.isfile(dest))
             content = open(dest).read()
             self.assertEqual(content, 'Hello world')
 
     def test_replace_file_with_symlink(self):
         # If symlinks are supported, an existing file should be replaced by a
         # symlink.
@@ -301,17 +301,17 @@ class TestAbsoluteSymlinkFile(TestWithTm
             pass
 
         s = AbsoluteSymlinkFile(source)
         s.copy(dest, skip_if_older=False)
 
         if self.symlink_supported:
             self.assertTrue(os.path.islink(dest))
             link = os.readlink(dest)
-            self.assertEqual(link, source)
+            self.assertEqual(link, 'test_path')
         else:
             self.assertTrue(os.path.isfile(dest))
             content = open(dest).read()
             self.assertEqual(content, 'source')
 
     def test_replace_symlink(self):
         if not self.symlink_supported:
             return
@@ -325,37 +325,37 @@ class TestAbsoluteSymlinkFile(TestWithTm
         os.symlink(self.tmppath('bad'), dest)
         self.assertTrue(os.path.islink(dest))
 
         s = AbsoluteSymlinkFile(source)
         self.assertTrue(s.copy(dest))
 
         self.assertTrue(os.path.islink(dest))
         link = os.readlink(dest)
-        self.assertEqual(link, source)
+        self.assertEqual(link, 'source')
 
     def test_noop(self):
         if not hasattr(os, 'symlink'):
             return
 
         source = self.tmppath('source')
         dest = self.tmppath('dest')
 
         with open(source, 'a'):
             pass
 
-        os.symlink(source, dest)
+        os.symlink('source', dest)
         link = os.readlink(dest)
-        self.assertEqual(link, source)
+        self.assertEqual(link, 'source')
 
         s = AbsoluteSymlinkFile(source)
         self.assertFalse(s.copy(dest))
 
         link = os.readlink(dest)
-        self.assertEqual(link, source)
+        self.assertEqual(link, 'source')
 
 class TestPreprocessedFile(TestWithTmpDir):
     def test_preprocess(self):
         '''
         Test that copying the file invokes the preprocessor
         '''
         src = self.tmppath('src')
         dest = self.tmppath('dest')