Merge pull request #614 from smira/600-fix-double-mirror-update

Fix bug with `PoolPath` field being overwritten on mirror update

aptly/interfaces.go

@@ -23,7 +23,7 @@ type PackagePool interface {
 	//
 	// if poolPath is empty, poolPath is generated automatically based on checksum info (if available)
 	// in any case, if function returns true, it also fills back checksums with complete information about the file in the pool
-	Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage ChecksumStorage) (bool, error)
+	Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage ChecksumStorage) (string, bool, error)
 	// Import copies file into package pool
 	//
 	// - srcPath is full path to source file as it is now

deb/package.go

@@ -629,14 +629,15 @@ type PackageDownloadTask struct {
 func (p *Package) DownloadList(packagePool aptly.PackagePool, checksumStorage aptly.ChecksumStorage) (result []PackageDownloadTask, err error) {
 	result = make([]PackageDownloadTask, 0, 1)
 
-	for idx, f := range p.Files() {
+	files := p.Files()
-		verified, err := f.Verify(packagePool, checksumStorage)
+	for idx := range files {
+		verified, err := files[idx].Verify(packagePool, checksumStorage)
 		if err != nil {
 			return nil, err
 		}
 
 		if !verified {
-			result = append(result, PackageDownloadTask{File: &p.Files()[idx]})
+			result = append(result, PackageDownloadTask{File: &files[idx]})
 		}
 	}
 

deb/package_files.go

@@ -27,7 +27,12 @@ type PackageFile struct {
 
 // Verify that package file is present and correct
 func (f *PackageFile) Verify(packagePool aptly.PackagePool, checksumStorage aptly.ChecksumStorage) (bool, error) {
-	return packagePool.Verify(f.PoolPath, f.Filename, &f.Checksums, checksumStorage)
+	generatedPoolPath, exists, err := packagePool.Verify(f.PoolPath, f.Filename, &f.Checksums, checksumStorage)
+	if exists && err == nil {
+		f.PoolPath = generatedPoolPath
+	}
+
+	return exists, err
 }
 
 // GetPoolPath returns path to the file in the pool

files/package_pool.go

@@ -162,7 +162,7 @@ func (pool *PackagePool) ensureChecksums(poolPath, fullPoolPath string, checksum
 //
 // if poolPath is empty, poolPath is generated automatically based on checksum info (if available)
 // in any case, if function returns true, it also fills back checksums with complete information about the file in the pool
-func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage aptly.ChecksumStorage) (bool, error) {
+func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.ChecksumInfo, checksumStorage aptly.ChecksumStorage) (string, bool, error) {
 	possiblePoolPaths := []string{}
 
 	if poolPath != "" {
@@ -172,7 +172,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec
 		if checksums.SHA256 != "" {
 			modernPath, err := pool.buildPoolPath(basename, checksums)
 			if err != nil {
-				return false, err
+				return "", false, err
 			}
 			possiblePoolPaths = append(possiblePoolPaths, modernPath)
 		}
@@ -180,7 +180,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec
 		if pool.supportLegacyPaths && checksums.MD5 != "" {
 			legacyPath, err := pool.LegacyPath(basename, checksums)
 			if err != nil {
-				return false, err
+				return "", false, err
 			}
 			possiblePoolPaths = append(possiblePoolPaths, legacyPath)
 		}
@@ -193,7 +193,7 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec
 		if err != nil {
 			if !os.IsNotExist(err) {
 				// unable to stat target location?
-				return false, err
+				return "", false, err
 			}
 			// doesn't exist, skip it
 			continue
@@ -208,21 +208,21 @@ func (pool *PackagePool) Verify(poolPath, basename string, checksums *utils.Chec
 		targetChecksums, err = pool.ensureChecksums(path, fullPoolPath, checksumStorage)
 
 		if err != nil {
-			return false, err
+			return "", false, err
 		}
 
 		if checksums.MD5 != "" && targetChecksums.MD5 != checksums.MD5 ||
 			checksums.SHA256 != "" && targetChecksums.SHA256 != checksums.SHA256 {
 			// wrong file?
-			return false, nil
+			return "", false, nil
 		}
 
 		// fill back checksums
 		*checksums = *targetChecksums
-		return true, nil
+		return path, true, nil
 	}
 
-	return false, nil
+	return "", false, nil
 }
 
 // Import copies file into package pool

files/package_pool_test.go

@@ -155,7 +155,8 @@ func (s *PackagePoolSuite) TestImportLegacy(c *C) {
 func (s *PackagePoolSuite) TestVerifyLegacy(c *C) {
 	s.checksum.Size = 2738
 	// file doesn't exist yet
-	exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	path, exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(path, Equals, "")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, false)
 
@@ -164,7 +165,8 @@ func (s *PackagePoolSuite) TestVerifyLegacy(c *C) {
 	c.Assert(err, IsNil)
 
 	// check existence (and fills back checksum)
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	path, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(path, Equals, "00/35/libboost-program-options-dev_1.49.0.1_i386.deb")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c")
@@ -172,7 +174,8 @@ func (s *PackagePoolSuite) TestVerifyLegacy(c *C) {
 
 func (s *PackagePoolSuite) TestVerify(c *C) {
 	// file doesn't exist yet
-	exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err := s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, "")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, false)
 
@@ -182,20 +185,23 @@ func (s *PackagePoolSuite) TestVerify(c *C) {
 	c.Check(path, Equals, "c7/6b/4bd12fd92e4dfe1b55b18a67a669_libboost-program-options-dev_1.49.0.1_i386.deb")
 
 	// check existence
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, ppath)
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c")
 
 	// check existence with fixed path
-	exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, path)
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	c.Check(s.checksum.SHA512, Equals, "d7302241373da972aa9b9e71d2fd769b31a38f71182aa71bc0d69d090d452c69bb74b8612c002ccf8a89c279ced84ac27177c8b92d20f00023b3d268e6cec69c")
 
 	// check existence, but with missing checksum
 	s.checksum.SHA512 = ""
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, path)
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	// checksum is filled back based on checksum storage
@@ -205,7 +211,8 @@ func (s *PackagePoolSuite) TestVerify(c *C) {
 	ck := utils.ChecksumInfo{
 		Size: s.checksum.Size,
 	}
-	exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs)
+	ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs)
+	c.Check(ppath, Equals, path)
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	// checksum is filled back based on checksum storage
@@ -213,14 +220,16 @@ func (s *PackagePoolSuite) TestVerify(c *C) {
 
 	// check existence, with wrong checksum info but correct path and size available
 	ck.SHA256 = "abc"
-	exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs)
+	ppath, exists, err = s.pool.Verify(path, filepath.Base(s.debFile), &ck, s.cs)
+	c.Check(ppath, Equals, "")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, false)
 
 	// check existence, with missing checksum and no info in checksum storage
 	delete(s.cs.(*mockChecksumStorage).store, path)
 	s.checksum.SHA512 = ""
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, path)
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, true)
 	// checksum is filled back based on re-calculation
@@ -228,12 +237,14 @@ func (s *PackagePoolSuite) TestVerify(c *C) {
 
 	// check existence, with wrong size
 	s.checksum.Size = 13455
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &s.checksum, s.cs)
+	c.Check(ppath, Equals, "")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, false)
 
 	// check existence, with empty checksum info
-	exists, err = s.pool.Verify("", filepath.Base(s.debFile), &utils.ChecksumInfo{}, s.cs)
+	ppath, exists, err = s.pool.Verify("", filepath.Base(s.debFile), &utils.ChecksumInfo{}, s.cs)
+	c.Check(ppath, Equals, "")
 	c.Check(err, IsNil)
 	c.Check(exists, Equals, false)
 }

system/t06_publish/PublishSnapshot37Test_gold

@@ -0,0 +1,13 @@
+Loading packages...
+Generating metadata files and linking package files...
+Finalizing metadata files...
+Signing file 'Release' with gpg, please enter your passphrase when prompted:
+Clearsigning file 'Release' with gpg, please enter your passphrase when prompted:
+
+Snapshot wheezy has been successfully published.
+Please setup your webserver to serve directory '${HOME}/.aptly/public' with autoindexing.
+Now you can add following line to apt sources:
+  deb http://your-server/ wheezy main
+Don't forget to add your GPG key to apt with apt-key.
+
+You can also use `aptly serve` to publish your repositories over HTTP quickly.

system/t06_publish/snapshot.py

@@ -1001,3 +1001,18 @@ class PublishSnapshot36Test(BaseTest):
         self.check_not_exists('public/dists/maverick/main/Contents-i386.gz')
         self.check_exists('public/dists/maverick/main/binary-amd64/Release')
         self.check_not_exists('public/dists/maverick/main/Contents-amd64.gz')
+
+
+class PublishSnapshot37Test(BaseTest):
+    """
+    publish snapshot: mirror with double mirror update
+    """
+    fixtureGpg = True
+    fixtureCmds = [
+        "aptly -architectures=i386,amd64 mirror create -keyring=aptlytest.gpg -filter='$$Source (gnupg)' -with-udebs wheezy http://mirror.yandex.ru/debian/ wheezy main non-free",
+        "aptly mirror update -keyring=aptlytest.gpg wheezy",
+        "aptly mirror update -keyring=aptlytest.gpg wheezy",
+        "aptly snapshot create wheezy from mirror wheezy",
+    ]
+    runCmd = "aptly publish snapshot -keyring=${files}/aptly.pub -secret-keyring=${files}/aptly.sec wheezy"
+    gold_processor = BaseTest.expand_environ