tests: check hardlinking in case of local download from Git working copy

The download recipe relies on buildout's download, which uses hardlinking for
performance reasons in 2 cases: downloading from cache or from local file.

This means 2 things:
- the user shall not modify downloaded files without first making sure
  that st_nlink == 1
- the source file shall not be modified in-place, which is reasable to assume
  for both cache and local file

In-place modification of files is more and more rare because it has many
drawbacks and it tends to be limited to things like logs and databases.
This commit adds a test to check that Git does not do that when managing
working copies: and actually, this may be our only use case of local download.

This commit does not mean that the current way of hardlinking is fully fine.
There remains at least the issue that file permissions may be changed at the
end of the recipe, either with `shared=true` or with `mode` option.

README.rst

@@ -248,6 +248,10 @@ Simplest usage is to only specify a URL::
 
 The file is downloaded to ``parts/<section_name>/<section_name>``.
 
+Because the destination file may be hardlinked (e.g. download from cache
+or from local file), it shall not be modified in-place without first making
+sure that ``st_nlink`` is 1.
+
 option: filename
 ----------------
 

slapos/recipe/build/tests.py

@@ -7,19 +7,18 @@ import shutil
 import tempfile
 import unittest
 import zc.buildout.testing
+from zc.buildout.download import check_md5sum, ChecksumError
 from zc.buildout.testing import buildoutTearDown
 from contextlib import contextmanager
 from functools import wraps
 from subprocess import check_call, check_output, CalledProcessError, STDOUT
-from ..gitclone import GIT_CLONE_ERROR_MESSAGE, GIT_CLONE_CACHE_ERROR_MESSAGE
-from .. import make_read_only_recursively
+from .. import download, gitclone, make_read_only_recursively
 
 
 optionflags = (doctest.ELLIPSIS | doctest.NORMALIZE_WHITESPACE)
 
 GIT_REPOSITORY = 'https://lab.nexedi.com/nexedi/slapos.recipe.build.git'
 BAD_GIT_REPOSITORY = 'http://git.erp5.org/repos/nowhere'
-REVISION = '2566127'
 
 def setUp(test):
   # XXX side effect. Disable libnetworkcache because buildout testing
@@ -49,11 +48,59 @@ def with_buildout(wrapped):
       buildoutTearDown(self)
   return wraps(wrapped)(wrapper)
 
-class GitCloneNonInformativeTests(unittest.TestCase):
+
+class TestCase(unittest.TestCase):
 
   def setUp(self):
     self.dir = os.path.realpath(tempfile.mkdtemp())
+    self.addCleanup(shutil.rmtree, self.dir)
     self.parts_directory_path = os.path.join(self.dir, 'test_parts')
+    self.buildout = {
+        'buildout': {
+            'parts-directory': self.parts_directory_path,
+            'directory': self.dir,
+         }
+    }
+
+  def makeGitCloneRecipe(self, options={}):
+    options.setdefault('repository', GIT_REPOSITORY)
+    return gitclone.Recipe(self.buildout, "working_copy", options)
+
+
+class DownloadTests(TestCase):
+
+  def test_hardlink(self):
+    filename = 'setup.py'
+    v0 = '0.50~'
+    h0 = '3365d85985af7421eff978b09682d0c8'
+    v1 = '0.50'
+    h1 = '6673e2f48d641d516f78cbddb281608e'
+    options = {'revision': v0}
+    self.makeGitCloneRecipe(options).install()
+    working_copy = options['location']
+    url = os.path.join(working_copy, filename)
+    options = {
+      'url': url,
+      'md5sum': h0,
+      'filename': filename,
+    }
+    download.Recipe(self.buildout, v0, options).install()
+    p0 = options['target']
+    self.assertTrue(os.path.samefile(p0, url))
+    check_call(('git', 'checkout', v1), cwd=working_copy)
+    self.assertFalse(os.path.samefile(p0, url))
+    options['md5sum'] = h1
+    download.Recipe(self.buildout, v1, options).install()
+    p1 = options['target']
+    self.assertTrue(check_md5sum(p0, h0))
+    self.assertTrue(check_md5sum(p1, h1))
+    self.assertTrue(os.path.samefile(p1, url))
+    open(p1, 'w').close()
+    self.assertRaises(ChecksumError,
+      download.Recipe(self.buildout, 'fail', options).install)
+
+
+class GitCloneNonInformativeTests(TestCase):
 
   def setUpParentRepository(self):
     """
@@ -111,7 +158,6 @@ class GitCloneNonInformativeTests(unittest.TestCase):
     self.attachSubmoduleToParent()
 
   def tearDown(self):
-    shutil.rmtree(self.dir)
     for var in list(os.environ):
       if var.startswith('SRB_'):
         del os.environ[var]
@@ -159,48 +205,30 @@ class GitCloneNonInformativeTests(unittest.TestCase):
     self.assertEqual(self.readFile(parent_local_change_path), 'foo')
     self.assertEqual(self.readFile(submodule_local_change_path), 'bar')
 
-  def makeGitCloneRecipe(self, options):
-    from slapos.recipe.gitclone import Recipe
-    bo = {
-        'buildout': {
-            'parts-directory': self.parts_directory_path,
-            'directory': self.dir,
-         }
-    }
-    default_options = {
-      'repository': GIT_REPOSITORY
-      }
-    default_options.update(**options)
-    return Recipe(bo, 'test', default_options)
-
   def test_using_download_cache_if_git_fails(self):
     recipe = self.makeGitCloneRecipe({"use-cache": "true",
                                       "repository": BAD_GIT_REPOSITORY})
     with chdir(self.dir), \
          self.assertRaises(zc.buildout.UserError) as cm:
       recipe.install()
-    self.assertEqual(str(cm.exception), GIT_CLONE_CACHE_ERROR_MESSAGE)
+    self.assertEqual(str(cm.exception), gitclone.GIT_CLONE_CACHE_ERROR_MESSAGE)
 
   def test_not_using_download_cache_if_forbidden(self):
     recipe = self.makeGitCloneRecipe({"repository": BAD_GIT_REPOSITORY})
     with chdir(self.dir), \
          self.assertRaises(zc.buildout.UserError) as cm:
       recipe.install()
-    self.assertEqual(str(cm.exception), GIT_CLONE_ERROR_MESSAGE)
+    self.assertEqual(str(cm.exception), gitclone.GIT_CLONE_ERROR_MESSAGE)
 
   def test_cleanup_of_pyc_files(self):
-    recipe = self.makeGitCloneRecipe({})
-    recipe.install()
-    git_repository_path = os.path.join(self.parts_directory_path, "test")
-    self.assertTrue(os.path.exists(git_repository_path))
-    bad_file_path = os.path.join(git_repository_path, "foo.pyc")
-    bade_file = open(bad_file_path, 'w')
-    bade_file.close()
+    self.makeGitCloneRecipe().install()
+    working_copy = os.path.join(self.parts_directory_path, "working_copy")
+    bad_file_path = os.path.join(working_copy, "foo.pyc")
+    open(bad_file_path, 'w').close()
     self.assertTrue(os.path.exists(bad_file_path))
     # install again and make sure pyc file is removed
-    recipe = self.makeGitCloneRecipe({})
-    recipe.update()
-    self.assertTrue(os.path.exists(git_repository_path))
+    self.makeGitCloneRecipe().update()
+    self.assertTrue(os.path.exists(working_copy))
     self.assertFalse(os.path.exists(bad_file_path), "pyc file not removed")
 
   @with_buildout
@@ -437,51 +465,36 @@ repository = %s
     self.assertEqual(file_changed, 'A  file2.py')
 
   def test_ignore_ssl_certificate(self, ignore_ssl_certificate=True):
-    import slapos.recipe.gitclone
-
     # Monkey patch check_call
-    original_check_call = slapos.recipe.gitclone.check_call
+    original_check_call = gitclone.check_call
     check_call_parameter_list = []
     def patch_check_call(*args, **kw):
       check_call_parameter_list.extend((args, kw))
       original_check_call(args[0])
-    slapos.recipe.gitclone.check_call = patch_check_call
-
-    bo = {
-        'buildout': {
-            'parts-directory': self.parts_directory_path,
-            'directory': self.dir,
-         }
-    }
-    options = {
-      'repository': GIT_REPOSITORY,
-      "ignore-ssl-certificate": str(ignore_ssl_certificate).lower(),
-    }
-    recipe = slapos.recipe.gitclone.Recipe(bo, 'test', options)
-
-    recipe.install()
+    try:
+      gitclone.check_call = patch_check_call
 
-    # Check git clone parameters
-    _ = self.assertIn if ignore_ssl_certificate else self.assertNotIn
-    _("http.sslVerify=false", check_call_parameter_list[0][0])
+      self.makeGitCloneRecipe({
+        "ignore-ssl-certificate": str(ignore_ssl_certificate).lower(),
+      }).install()
 
-    # Restore original check_call method
-    slapos.recipe.gitclone.check_call = original_check_call
+      # Check git clone parameters
+      _ = self.assertIn if ignore_ssl_certificate else self.assertNotIn
+      _("http.sslVerify=false", check_call_parameter_list[0][0])
+    finally:
+      gitclone.check_call = original_check_call
 
   def test_ignore_ssl_certificate_false(self):
     self.test_ignore_ssl_certificate(ignore_ssl_certificate=False)
 
   def test_clone_submodules_by_default(self, ignore_cloning_submodules=False):
     self.createRepositoriesAndConnect()
-    recipe = self.makeGitCloneRecipe(
+    self.makeGitCloneRecipe(
       {'repository': self.project_dir,
         'ignore-cloning-submodules': str(ignore_cloning_submodules).lower()}
-    )
-    recipe.install()
-    main_repo_path = os.path.join(self.parts_directory_path, "test")
-    self.assertTrue(os.path.exists(main_repo_path))
-    submodule_repo_path = os.path.join(main_repo_path, 'dir1',
-                                       'submodule_repo')
+    ).install()
+    main_repo_path = os.path.join(self.parts_directory_path, "working_copy")
+    submodule_repo_path = os.path.join(main_repo_path, 'dir1', 'submodule_repo')
     # Check if the folder exists
     self.assertTrue(os.path.exists(main_repo_path))
     # Check is there is anything in submodule repository path
@@ -562,6 +575,7 @@ class MakeReadOnlyTests(unittest.TestCase):
     make_read_only_recursively(self.tmp_dir)
     self.assertRaises(IOError, open, os.path.join(self.tmp_dir, 'folder', 'symlink'), 'w')
 
+
 MD5SUM = []
 
 def md5sum(m):
@@ -596,6 +610,7 @@ def test_suite():
               ] + renormalizing_patters),
           globs={'MD5SUM': MD5SUM},
           ),
+      unittest.makeSuite(DownloadTests),
       unittest.makeSuite(GitCloneNonInformativeTests),
       unittest.makeSuite(MakeReadOnlyTests),
       ))