Merge lp:~mvo/snappy/snappy-lp1449032-poor-mans-rsync into lp:~snappy-dev/snappy/snappy-moved-to-github

[Michael Vogt]

Only update changed files in SyncBootloader().

This is lp:~mvo/snappy/snappy-lp1449032-poor-mans-rsync-15.04 for trunk

This branch changes SyncBootloader() to only update files that actually are different instead of just copying everything everytime. It also fixes some visual progress artefacts. Please let me know if you prefer that in a different branch.

[John Lenton (chipaca)]

review: Approve

[Snappy Tarmac (snappydevtarmac)]

Attempt to merge into lp:snappy failed due to conflicts:

text conflict in helpers/helpers.go

[lp:~mvo/snappy/snappy-lp1449032-poor-mans-rsync]

484. By Michael Vogt on 2015-06-08

merged lp:snappy and resolved conflicts

[Michael Vogt (mvo)]

review: Approve

=== modified file 'helpers/helpers.go'
--- helpers/helpers.go	2015-06-04 12:55:54 +0000
+++ helpers/helpers.go	2015-06-08 14:25:18 +0000
@@ -436,3 +436,55 @@
 		"SNAPP_APP_USER_DATA_PATH={{.Home}}{{.AppPath}}",
 	})
 }
+
+// RSyncWithDelete syncs srcDir to destDir
+func RSyncWithDelete(srcDirName, destDirName string) error {
+	// first remove everything thats not in srcdir
+	err := filepath.Walk(destDirName, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// relative to the root "destDirName"
+		relPath := path[len(destDirName):]
+		if !FileExists(filepath.Join(srcDirName, relPath)) {
+			if err := os.RemoveAll(path); err != nil {
+				return err
+			}
+			if info.IsDir() {
+				return filepath.SkipDir
+			}
+		}
+		return nil
+	})
+	if err != nil {
+		return err
+	}
+
+	// then copy or update the data from srcdir to destdir
+	err = filepath.Walk(srcDirName, func(path string, info os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+
+		// relative to the root "srcDirName"
+		relPath := path[len(srcDirName):]
+		if info.IsDir() {
+			return os.MkdirAll(filepath.Join(destDirName, relPath), info.Mode())
+		}
+		src := path
+		dst := filepath.Join(destDirName, relPath)
+		if !FilesAreEqual(src, dst) {
+			// XXX: we should (eventually) use CopyFile here,
+			//      but we need to teach it about preserving
+			//      of atime/mtime and permissions
+			output, err := exec.Command("cp", "-va", src, dst).CombinedOutput()
+			if err != nil {
+				fmt.Errorf("Failed to copy %s to %s (%s)", src, dst, output)
+			}
+		}
+		return nil
+	})
+
+	return err
+}

=== modified file 'helpers/helpers_test.go'
--- helpers/helpers_test.go	2015-06-04 12:55:54 +0000
+++ helpers/helpers_test.go	2015-06-08 14:25:18 +0000
@@ -21,6 +21,7 @@
 
 import (
 	"compress/gzip"
+	"fmt"
 	"io/ioutil"
 	"math/rand"
 	"os"
@@ -328,3 +329,104 @@
 	c.Assert(mknodWasCalled, Equals, true)
 
 }
+
+func makeTestFiles(c *C, srcDir, destDir string) {
+	// a new file
+	err := ioutil.WriteFile(filepath.Join(srcDir, "new"), []byte(nil), 0644)
+	c.Assert(err, IsNil)
+
+	// a existing file that needs update
+	err = ioutil.WriteFile(filepath.Join(destDir, "existing-update"), []byte("old-content"), 0644)
+	c.Assert(err, IsNil)
+	err = ioutil.WriteFile(filepath.Join(srcDir, "existing-update"), []byte("some-new-content"), 0644)
+	c.Assert(err, IsNil)
+
+	// existing file that needs no update
+	err = ioutil.WriteFile(filepath.Join(srcDir, "existing-unchanged"), []byte(nil), 0644)
+	c.Assert(err, IsNil)
+	err = exec.Command("cp", "-a", filepath.Join(srcDir, "existing-unchanged"), filepath.Join(destDir, "existing-unchanged")).Run()
+	c.Assert(err, IsNil)
+
+	// a file that needs removal
+	err = ioutil.WriteFile(filepath.Join(destDir, "to-be-deleted"), []byte(nil), 0644)
+	c.Assert(err, IsNil)
+}
+
+func compareDirs(c *C, srcDir, destDir string) {
+	d1, err := exec.Command("ls", "-al", srcDir).CombinedOutput()
+	c.Assert(err, IsNil)
+	d2, err := exec.Command("ls", "-al", destDir).CombinedOutput()
+	c.Assert(err, IsNil)
+	c.Assert(string(d1), Equals, string(d2))
+	// ensure content got updated
+	c1, err := exec.Command("sh", "-c", fmt.Sprintf("find %s -type f |xargs cat", srcDir)).CombinedOutput()
+	c.Assert(err, IsNil)
+	c2, err := exec.Command("sh", "-c", fmt.Sprintf("find %s -type f |xargs cat", destDir)).CombinedOutput()
+	c.Assert(err, IsNil)
+	c.Assert(string(c1), Equals, string(c2))
+}
+
+func (ts *HTestSuite) TestSyncDirs(c *C) {
+
+	for _, l := range [][2]string{
+		[2]string{"src-short", "dst-loooooooooooong"},
+		[2]string{"src-loooooooooooong", "dst-short"},
+		[2]string{"src-eq", "dst-eq"},
+	} {
+
+		// ensure we have src, dest dirs with different length
+		srcDir := filepath.Join(c.MkDir(), l[0])
+		err := os.MkdirAll(srcDir, 0755)
+		c.Assert(err, IsNil)
+		destDir := filepath.Join(c.MkDir(), l[1])
+		err = os.MkdirAll(destDir, 0755)
+		c.Assert(err, IsNil)
+
+		// add a src subdir
+		subdir := filepath.Join(srcDir, "subdir")
+		err = os.Mkdir(subdir, 0755)
+		c.Assert(err, IsNil)
+		makeTestFiles(c, subdir, destDir)
+
+		// add a dst subdir that needs to get deleted
+		subdir2 := filepath.Join(destDir, "to-be-deleted-subdir")
+		err = os.Mkdir(subdir2, 0755)
+		subdir3 := filepath.Join(subdir2, "to-be-deleted-sub-subdir")
+		err = os.Mkdir(subdir3, 0755)
+
+		// and a toplevel
+		makeTestFiles(c, srcDir, destDir)
+
+		// do it
+		err = RSyncWithDelete(srcDir, destDir)
+		c.Assert(err, IsNil)
+
+		// ensure meta-data is identical
+		compareDirs(c, srcDir, destDir)
+		compareDirs(c, filepath.Join(srcDir, "subdir"), filepath.Join(destDir, "subdir"))
+	}
+}
+
+func (ts *HTestSuite) TestSyncDirFails(c *C) {
+	srcDir := c.MkDir()
+	err := os.MkdirAll(srcDir, 0755)
+	c.Assert(err, IsNil)
+
+	destDir := c.MkDir()
+	err = os.MkdirAll(destDir, 0755)
+	c.Assert(err, IsNil)
+
+	err = ioutil.WriteFile(filepath.Join(destDir, "meep"), []byte(nil), 0644)
+	c.Assert(err, IsNil)
+
+	// ensure remove fails
+	err = os.Chmod(destDir, 0100)
+	c.Assert(err, IsNil)
+	// make tempdir cleanup work again
+	defer os.Chmod(destDir, 0755)
+
+	// do it
+	err = RSyncWithDelete(srcDir, destDir)
+	c.Check(err, NotNil)
+	c.Check(err, ErrorMatches, ".*permission denied.*")
+}

=== modified file 'partition/bootloader_uboot.go'
--- partition/bootloader_uboot.go	2015-05-29 09:55:15 +0000
+++ partition/bootloader_uboot.go	2015-06-08 14:25:18 +0000
@@ -221,10 +221,7 @@
 	srcDir := u.currentBootPath
 	destDir := u.otherBootPath
 
-	// always start from scratch: all files here are owned by us.
-	os.RemoveAll(destDir)
-
-	return runCommand("/bin/cp", "-a", srcDir, destDir)
+	return helpers.RSyncWithDelete(srcDir, destDir)
 }
 
 func (u *uboot) HandleAssets() (err error) {

=== modified file 'partition/partition.go'
--- partition/partition.go	2015-05-29 09:55:15 +0000
+++ partition/partition.go	2015-06-08 14:25:18 +0000
@@ -814,6 +814,9 @@
 		return err
 	}
 
+	// XXX: first toggle roofs and then handle assets? that seems
+	//      wrong given that handleAssets may fails and we will
+	//      knowingly boot into a broken system
 	err = p.RunWithOther(RW, func(otherRoot string) (err error) {
 		return bootloader.ToggleRootFS()
 	})

=== modified file 'progress/progress.go'
--- progress/progress.go	2015-05-15 13:33:27 +0000
+++ progress/progress.go	2015-06-08 14:25:18 +0000
@@ -129,8 +129,9 @@
 // Finished stops displaying the progress
 func (t *TextProgress) Finished() {
 	if t.pbar != nil {
-		t.pbar.FinishPrint("Done")
+		t.pbar.Finish()
 	}
+	fmt.Println("Done")
 }
 
 // Write is there so that progress can implment a Writer and can be
@@ -143,7 +144,10 @@
 // that have a unknown duration
 func (t *TextProgress) Spin(msg string) {
 	states := `|/-\`
-	fmt.Printf("\r%s[%c]", msg, states[t.spinStep])
+
+	// clear until end of line
+	clearUntilEOL := "\033[K"
+	fmt.Printf("\r%s[%c]%s", msg, states[t.spinStep], clearUntilEOL)
 	t.spinStep++
 	if t.spinStep >= len(states) {
 		t.spinStep = 0

=== modified file 'progress/progress_test.go'
--- progress/progress_test.go	2015-06-02 20:53:10 +0000
+++ progress/progress_test.go	2015-06-08 14:25:18 +0000
@@ -60,7 +60,7 @@
 	f.Seek(0, 0)
 	progress, err := ioutil.ReadAll(f)
 	c.Assert(err, IsNil)
-	c.Assert(string(progress), Equals, "\rm[|]\rm[/]\rm[-]\rm[\\]\rm[|]\rm[/]")
+	c.Assert(string(progress), Equals, "\rm[|]\x1b[K\rm[/]\x1b[K\rm[-]\x1b[K\rm[\\]\x1b[K\rm[|]\x1b[K\rm[/]\x1b[K")
 }
 
 func (ts *ProgressTestSuite) testAgreed(answer string, value bool, c *C) {

=== modified file 'snappy/systemimage.go'
--- snappy/systemimage.go	2015-05-20 17:24:29 +0000
+++ snappy/systemimage.go	2015-06-08 14:25:18 +0000
@@ -198,6 +198,9 @@
 
 	// Ensure there is always a kernel + initrd to boot with, even
 	// if the update does not provide new versions.
+	if pb != nil {
+		pb.Notify("Syncing boot files")
+	}
 	err = s.partition.SyncBootloaderFiles()
 	if err != nil {
 		return "", err
@@ -226,6 +229,13 @@
 		return "", err
 	}
 
+	// XXX: ToggleNextBoot() calls handleAssets() (but not SyncBootloader
+	//      files :/) - handleAssets() may copy kernel/initramfs to the
+	//      sync mounted /boot/uboot, so its very slow, tell the user
+	//      at least that something is going on
+	if pb != nil {
+		pb.Notify("Updating boot files")
+	}
 	if err = s.partition.ToggleNextBoot(); err != nil {
 		return "", err
 	}

=== modified file 'snappy/systemimage_native.go'
--- snappy/systemimage_native.go	2015-05-15 13:33:27 +0000
+++ snappy/systemimage_native.go	2015-06-08 14:25:18 +0000
@@ -158,6 +158,8 @@
 			pb.Set(genericData.Now)
 		}
 	}
+	// ugly: avoid Spin() artifacts
+	pb.Notify("\nApply done")
 
 	if err := scanner.Err(); err != nil {
 		return err