From 07b671aa4227de2f19674d6e460d479ab96b07a6 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 12:37:47 +0200 Subject: [PATCH 1/9] Clone with tags as the explicit target Avoids accidentally cloning a branch with the same name as that would take preference for the --branch option of git clone --- easybuild/tools/filetools.py | 2 +- test/framework/filetools.py | 20 +++++++++++++------- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 1cb65f826f..7c1e201674 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2472,7 +2472,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config): clone_cmd = ['git', 'clone'] if tag: - clone_cmd.extend(['--branch', tag]) + clone_cmd.extend(['--branch', 'refs/tags/' + tag]) if recursive: clone_cmd.append('--recursive') diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 444a6986e3..58cfbb3c6a 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2491,16 +2491,22 @@ def test_get_source_tarball_from_git(self): git_config = { 'repo_name': 'testrepository', 'url': 'https://github.com/easybuilders', - 'tag': 'main', + 'tag': 'tag_for_tests', } target_dir = os.path.join(self.test_prefix, 'target') try: ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) # (only) tarball is created in specified target dir - self.assertTrue(os.path.isfile(os.path.join(target_dir, 'test.tar.gz'))) + test_file = os.path.join(target_dir, 'test.tar.gz') + self.assertTrue(os.path.isfile(test_file)) self.assertEqual(os.listdir(target_dir), ['test.tar.gz']) + # Check that we indeed downloaded the tag and not a branch + extracted_dir = tempfile.mkdtemp(prefix='extracted_dir') + target_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) + self.assertTrue(os.path.isfile(os.path.join(target_dir, 'this-is-a-tag.txt'))) + del git_config['tag'] git_config['commit'] = '8456f86' ft.get_source_tarball_from_git('test2.tar.gz', target_dir, git_config) @@ -2516,7 +2522,7 @@ def test_get_source_tarball_from_git(self): git_config = { 'repo_name': 'testrepository', 'url': 'git@github.com:easybuilders', - 'tag': 'master', + 'tag': 'tag_for_tests', } args = ['test.tar.gz', self.test_prefix, git_config] @@ -2570,10 +2576,10 @@ def run_check(): git_config = { 'repo_name': 'testrepository', 'url': 'git@github.com:easybuilders', - 'tag': 'master', + 'tag': 'tag_for_tests', } expected = '\n'.join([ - r' running command "git clone --branch master git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --branch refs/tags/tag_for_tests git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2582,7 +2588,7 @@ def run_check(): git_config['recursive'] = True expected = '\n'.join([ - r' running command "git clone --branch master --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2591,7 +2597,7 @@ def run_check(): git_config['keep_git_dir'] = True expected = '\n'.join([ - r' running command "git clone --branch master --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz testrepository"', r" \(in .*/tmp.*\)", From 956fa9c49a7572fdafc810a2f48262bc7eeacc19 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 12:46:00 +0200 Subject: [PATCH 2/9] Speed up git checkouts Use shallow checkouts if .git folder is not required Don't download submodules if we do that again for a potentially other version --- easybuild/tools/filetools.py | 12 +++++++++--- test/framework/filetools.py | 8 ++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 7c1e201674..dfd2012e56 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2471,11 +2471,17 @@ def get_source_tarball_from_git(filename, targetdir, git_config): # compose 'git clone' command, and run it clone_cmd = ['git', 'clone'] + if not keep_git_dir: + # Speed up cloning by only fetching the most recent commit, not the whole history + # When we don't want to keep the .git folder there won't be a difference in the result + clone_cmd.extend(['--depth', '1']) + if tag: clone_cmd.extend(['--branch', 'refs/tags/' + tag]) - - if recursive: - clone_cmd.append('--recursive') + if recursive: + clone_cmd.append('--recursive') + else: + clone_cmd.append('--no-checkout') # We do that manually below clone_cmd.append('%s/%s.git' % (url, repo_name)) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 58cfbb3c6a..6e4a9c06e5 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2579,7 +2579,7 @@ def run_check(): 'tag': 'tag_for_tests', } expected = '\n'.join([ - r' running command "git clone --branch refs/tags/tag_for_tests git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2588,7 +2588,7 @@ def run_check(): git_config['recursive'] = True expected = '\n'.join([ - r' running command "git clone --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2608,7 +2608,7 @@ def run_check(): del git_config['tag'] git_config['commit'] = '8456f86' expected = '\n'.join([ - r' running command "git clone --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --no-checkout git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "git checkout 8456f86 && git submodule update --init --recursive"', r" \(in testrepository\)", @@ -2619,7 +2619,7 @@ def run_check(): del git_config['recursive'] expected = '\n'.join([ - r' running command "git clone git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --no-checkout git@github.com:easybuilders/testrepository.git"', r" \(in .*/tmp.*\)", r' running command "git checkout 8456f86"', r" \(in testrepository\)", From d1fa2a4669112c4dcc86450ba1c2394668b3525e Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 12:50:12 +0200 Subject: [PATCH 3/9] Slightly enhance test Check for return value of get_source_tarball_from_git and use context manager --- test/framework/filetools.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 6e4a9c06e5..41c88b0afb 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2496,9 +2496,10 @@ def test_get_source_tarball_from_git(self): target_dir = os.path.join(self.test_prefix, 'target') try: - ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) # (only) tarball is created in specified target dir test_file = os.path.join(target_dir, 'test.tar.gz') + self.assertEqual(res, test_file) self.assertTrue(os.path.isfile(test_file)) self.assertEqual(os.listdir(target_dir), ['test.tar.gz']) @@ -2509,8 +2510,10 @@ def test_get_source_tarball_from_git(self): del git_config['tag'] git_config['commit'] = '8456f86' - ft.get_source_tarball_from_git('test2.tar.gz', target_dir, git_config) - self.assertTrue(os.path.isfile(os.path.join(target_dir, 'test2.tar.gz'))) + res = ft.get_source_tarball_from_git('test2.tar.gz', target_dir, git_config) + test_file = os.path.join(target_dir, 'test2.tar.gz') + self.assertEqual(res, test_file) + self.assertTrue(os.path.isfile(test_file)) self.assertEqual(sorted(os.listdir(target_dir)), ['test.tar.gz', 'test2.tar.gz']) except EasyBuildError as err: @@ -2559,13 +2562,10 @@ def test_get_source_tarball_from_git(self): def run_check(): """Helper function to run get_source_tarball_from_git & check dry run output""" - self.mock_stdout(True) - self.mock_stderr(True) - res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) - stdout = self.get_stdout() - stderr = self.get_stderr() - self.mock_stdout(False) - self.mock_stderr(False) + with self.mocked_stdout_stderr(): + res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + stdout = self.get_stdout() + stderr = self.get_stderr() self.assertEqual(stderr, '') regex = re.compile(expected) self.assertTrue(regex.search(stdout), "Pattern '%s' found in: %s" % (regex.pattern, stdout)) From 15799c0e33bb4a4629dca4baeeceec41a5285f5d Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 15:16:57 +0200 Subject: [PATCH 4/9] Move dry-run tests before real tests --- test/framework/filetools.py | 130 +++++++++++++++++++----------------- 1 file changed, 67 insertions(+), 63 deletions(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 41c88b0afb..245eab6981 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2488,71 +2488,8 @@ def test_diff_files(self): def test_get_source_tarball_from_git(self): """Test get_source_tarball_from_git function.""" - git_config = { - 'repo_name': 'testrepository', - 'url': 'https://github.com/easybuilders', - 'tag': 'tag_for_tests', - } target_dir = os.path.join(self.test_prefix, 'target') - try: - res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) - # (only) tarball is created in specified target dir - test_file = os.path.join(target_dir, 'test.tar.gz') - self.assertEqual(res, test_file) - self.assertTrue(os.path.isfile(test_file)) - self.assertEqual(os.listdir(target_dir), ['test.tar.gz']) - - # Check that we indeed downloaded the tag and not a branch - extracted_dir = tempfile.mkdtemp(prefix='extracted_dir') - target_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) - self.assertTrue(os.path.isfile(os.path.join(target_dir, 'this-is-a-tag.txt'))) - - del git_config['tag'] - git_config['commit'] = '8456f86' - res = ft.get_source_tarball_from_git('test2.tar.gz', target_dir, git_config) - test_file = os.path.join(target_dir, 'test2.tar.gz') - self.assertEqual(res, test_file) - self.assertTrue(os.path.isfile(test_file)) - self.assertEqual(sorted(os.listdir(target_dir)), ['test.tar.gz', 'test2.tar.gz']) - - except EasyBuildError as err: - if "Network is down" in str(err): - print("Ignoring download error in test_get_source_tarball_from_git, working offline?") - else: - raise err - - git_config = { - 'repo_name': 'testrepository', - 'url': 'git@github.com:easybuilders', - 'tag': 'tag_for_tests', - } - args = ['test.tar.gz', self.test_prefix, git_config] - - for key in ['repo_name', 'url', 'tag']: - orig_value = git_config.pop(key) - if key == 'tag': - error_pattern = "Neither tag nor commit found in git_config parameter" - else: - error_pattern = "%s not specified in git_config parameter" % key - self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) - git_config[key] = orig_value - - git_config['commit'] = '8456f86' - error_pattern = "Tag and commit are mutually exclusive in git_config parameter" - self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) - del git_config['commit'] - - git_config['unknown'] = 'foobar' - error_pattern = "Found one or more unexpected keys in 'git_config' specification" - self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) - del git_config['unknown'] - - args[0] = 'test.txt' - error_pattern = "git_config currently only supports filename ending in .tar.gz" - self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) - args[0] = 'test.tar.gz' - # only test in dry run mode, i.e. check which commands would be executed without actually running them build_options = { 'extended_dry_run': True, @@ -2628,6 +2565,73 @@ def run_check(): ]) run_check() + # Test with real data + init_config() + git_config = { + 'repo_name': 'testrepository', + 'url': 'https://github.com/easybuilders', + 'tag': 'tag_for_tests', + } + + try: + res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + # (only) tarball is created in specified target dir + test_file = os.path.join(target_dir, 'test.tar.gz') + self.assertEqual(res, test_file) + self.assertTrue(os.path.isfile(test_file)) + self.assertEqual(os.listdir(target_dir), ['test.tar.gz']) + + # Check that we indeed downloaded the tag and not a branch + extracted_dir = tempfile.mkdtemp(prefix='extracted_dir') + target_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) + self.assertTrue(os.path.isfile(os.path.join(target_dir, 'this-is-a-tag.txt'))) + + del git_config['tag'] + git_config['commit'] = '8456f86' + res = ft.get_source_tarball_from_git('test2.tar.gz', target_dir, git_config) + test_file = os.path.join(target_dir, 'test2.tar.gz') + self.assertEqual(res, test_file) + self.assertTrue(os.path.isfile(test_file)) + self.assertEqual(sorted(os.listdir(target_dir)), ['test.tar.gz', 'test2.tar.gz']) + + except EasyBuildError as err: + if "Network is down" in str(err): + print("Ignoring download error in test_get_source_tarball_from_git, working offline?") + else: + raise err + + git_config = { + 'repo_name': 'testrepository', + 'url': 'git@github.com:easybuilders', + 'tag': 'tag_for_tests', + } + args = ['test.tar.gz', self.test_prefix, git_config] + + for key in ['repo_name', 'url', 'tag']: + orig_value = git_config.pop(key) + if key == 'tag': + error_pattern = "Neither tag nor commit found in git_config parameter" + else: + error_pattern = "%s not specified in git_config parameter" % key + self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) + git_config[key] = orig_value + + git_config['commit'] = '8456f86' + error_pattern = "Tag and commit are mutually exclusive in git_config parameter" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) + del git_config['commit'] + + git_config['unknown'] = 'foobar' + error_pattern = "Found one or more unexpected keys in 'git_config' specification" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) + del git_config['unknown'] + + args[0] = 'test.txt' + error_pattern = "git_config currently only supports filename ending in .tar.gz" + self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) + args[0] = 'test.tar.gz' + + def test_is_sha256_checksum(self): """Test for is_sha256_checksum function.""" a_sha256_checksum = '44332000aa33b99ad1e00cbd1a7da769220d74647060a10e807b916d73ea27bc' From 95e05c88d3089b48225d73bc036dd24d366b45cb Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 15:22:37 +0200 Subject: [PATCH 5/9] Fix line length --- test/framework/filetools.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 245eab6981..542a7c91e2 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2515,54 +2515,55 @@ def run_check(): 'url': 'git@github.com:easybuilders', 'tag': 'tag_for_tests', } + git_repo = {'git_repo': 'git@github.com:easybuilders/testrepository.git'} # Just to make the below shorter expected = '\n'.join([ - r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", - ]) + ]) % git_repo run_check() git_config['recursive'] = True expected = '\n'.join([ - r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests --recursive %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", - ]) + ]) % git_repo run_check() git_config['keep_git_dir'] = True expected = '\n'.join([ - r' running command "git clone --branch refs/tags/tag_for_tests --recursive git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --branch refs/tags/tag_for_tests --recursive %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz testrepository"', r" \(in .*/tmp.*\)", - ]) + ]) % git_repo run_check() del git_config['keep_git_dir'] del git_config['tag'] git_config['commit'] = '8456f86' expected = '\n'.join([ - r' running command "git clone --depth 1 --no-checkout git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --no-checkout %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "git checkout 8456f86 && git submodule update --init --recursive"', r" \(in testrepository\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", - ]) + ]) % git_repo run_check() del git_config['recursive'] expected = '\n'.join([ - r' running command "git clone --depth 1 --no-checkout git@github.com:easybuilders/testrepository.git"', + r' running command "git clone --depth 1 --no-checkout %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "git checkout 8456f86"', r" \(in testrepository\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", - ]) + ]) % git_repo run_check() # Test with real data @@ -2631,7 +2632,6 @@ def run_check(): self.assertErrorRegex(EasyBuildError, error_pattern, ft.get_source_tarball_from_git, *args) args[0] = 'test.tar.gz' - def test_is_sha256_checksum(self): """Test for is_sha256_checksum function.""" a_sha256_checksum = '44332000aa33b99ad1e00cbd1a7da769220d74647060a10e807b916d73ea27bc' From fafaf309a082fe9baf01d4006fd8034acef1f842 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 17:16:25 +0200 Subject: [PATCH 6/9] Fix the cloning of tags Can't use refs/tags/xxx for git clone so clone it assuming it is a tag and check afterwards. Fall back to fetching the full history and checking out the tag and submodules manually --- easybuild/tools/filetools.py | 27 +++++++++++++++++++++++---- test/framework/filetools.py | 26 ++++++++++++++++++-------- 2 files changed, 41 insertions(+), 12 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index dfd2012e56..df23ec331d 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2477,7 +2477,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config): clone_cmd.extend(['--depth', '1']) if tag: - clone_cmd.extend(['--branch', 'refs/tags/' + tag]) + clone_cmd.extend(['--branch', tag]) if recursive: clone_cmd.append('--recursive') else: @@ -2487,7 +2487,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config): tmpdir = tempfile.mkdtemp() cwd = change_dir(tmpdir) - run.run_cmd(' '.join(clone_cmd), log_all=True, log_ok=False, simple=False, regexp=False) + run.run_cmd(' '.join(clone_cmd), log_all=True, simple=True, regexp=False) # if a specific commit is asked for, check it out if commit: @@ -2495,14 +2495,33 @@ def get_source_tarball_from_git(filename, targetdir, git_config): if recursive: checkout_cmd.extend(['&&', 'git', 'submodule', 'update', '--init', '--recursive']) - run.run_cmd(' '.join(checkout_cmd), log_all=True, log_ok=False, simple=False, regexp=False, path=repo_name) + run.run_cmd(' '.join(checkout_cmd), log_all=True, simple=True, regexp=False, path=repo_name) + elif not build_option('extended_dry_run'): + # If we wanted to get a tag make sure we actually got a tag and not a branch with the same name + # This doesn't make sense in dry-run mode as we don't have anything to check + cmd = 'git describe --exact-match --tags HEAD' + # Note: Disable logging to also disable the error handling in run_cmd + (out, ec) = run.run_cmd(cmd, log_ok=False, log_all=False, regexp=False, path=repo_name) + if ec != 0 or tag not in out.splitlines(): + cmds = [] + if not keep_git_dir: + # Make the repo unshallow, same as git fetch --unshallow in git 1.8.3+ + # The first fetch seemingly does nothing, no idea why. + cmds.append('git fetch --depth=2147483647 && git fetch --depth=2147483647') + cmds.append('git checkout refs/tags/' + tag) + # Clean all untracked files, e.g. from left-over submodules + cmds.append('git clean --force -d -x') + if recursive: + cmds.append('git submodule update --init --recursive') + for cmd in cmds: + run.run_cmd(cmd, log_all=True, simple=True, regexp=False, path=repo_name) # create an archive and delete the git repo directory if keep_git_dir: tar_cmd = ['tar', 'cfvz', targetpath, repo_name] else: tar_cmd = ['tar', 'cfvz', targetpath, '--exclude', '.git', repo_name] - run.run_cmd(' '.join(tar_cmd), log_all=True, log_ok=False, simple=False, regexp=False) + run.run_cmd(' '.join(tar_cmd), log_all=True, simple=True, regexp=False) # cleanup (repo_name dir does not exist in dry run mode) change_dir(cwd) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 542a7c91e2..722fbf54da 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2517,7 +2517,7 @@ def run_check(): } git_repo = {'git_repo': 'git@github.com:easybuilders/testrepository.git'} # Just to make the below shorter expected = '\n'.join([ - r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests %(git_repo)s"', + r' running command "git clone --depth 1 --branch tag_for_tests %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2526,7 +2526,7 @@ def run_check(): git_config['recursive'] = True expected = '\n'.join([ - r' running command "git clone --depth 1 --branch refs/tags/tag_for_tests --recursive %(git_repo)s"', + r' running command "git clone --depth 1 --branch tag_for_tests --recursive %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz --exclude .git testrepository"', r" \(in .*/tmp.*\)", @@ -2535,7 +2535,7 @@ def run_check(): git_config['keep_git_dir'] = True expected = '\n'.join([ - r' running command "git clone --branch refs/tags/tag_for_tests --recursive %(git_repo)s"', + r' running command "git clone --branch tag_for_tests --recursive %(git_repo)s"', r" \(in .*/tmp.*\)", r' running command "tar cfvz .*/target/test.tar.gz testrepository"', r" \(in .*/tmp.*\)", @@ -2566,12 +2566,12 @@ def run_check(): ]) % git_repo run_check() - # Test with real data + # Test with real data. init_config() git_config = { 'repo_name': 'testrepository', 'url': 'https://github.com/easybuilders', - 'tag': 'tag_for_tests', + 'tag': 'branch_tag_for_test', } try: @@ -2581,11 +2581,21 @@ def run_check(): self.assertEqual(res, test_file) self.assertTrue(os.path.isfile(test_file)) self.assertEqual(os.listdir(target_dir), ['test.tar.gz']) + # Check that we indeed downloaded the right tag + extracted_dir = tempfile.mkdtemp(prefix='extracted_dir') + extracted_repo_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) + self.assertTrue(os.path.isfile(os.path.join(extracted_repo_dir, 'this-is-a-branch.txt'))) + os.remove(test_file) - # Check that we indeed downloaded the tag and not a branch + # use a tag that clashes with a branch name and make sure this is handled correctly + git_config['tag'] = 'tag_for_tests' + res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + self.assertEqual(res, test_file) + self.assertTrue(os.path.isfile(test_file)) + # Check that we indeed downloaded the tag and not the branch extracted_dir = tempfile.mkdtemp(prefix='extracted_dir') - target_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) - self.assertTrue(os.path.isfile(os.path.join(target_dir, 'this-is-a-tag.txt'))) + extracted_repo_dir = ft.extract_file(test_file, extracted_dir, change_into_dir=False) + self.assertTrue(os.path.isfile(os.path.join(extracted_repo_dir, 'this-is-a-tag.txt'))) del git_config['tag'] git_config['commit'] = '8456f86' From c352a9a588c964fd73e557c37f8584f77937886f Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Wed, 4 Aug 2021 17:36:40 +0200 Subject: [PATCH 7/9] Print a warning if the slow path is entered --- easybuild/tools/filetools.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index df23ec331d..983abdcb76 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2503,6 +2503,9 @@ def get_source_tarball_from_git(filename, targetdir, git_config): # Note: Disable logging to also disable the error handling in run_cmd (out, ec) = run.run_cmd(cmd, log_ok=False, log_all=False, regexp=False, path=repo_name) if ec != 0 or tag not in out.splitlines(): + print_warning('Tag %s was not downloaded in the first try due to %s/%s containing a branch' + ' with the same name. You might want to alert the maintainers of %s about that issue.', + tag, url, repo_name, repo_name) cmds = [] if not keep_git_dir: # Make the repo unshallow, same as git fetch --unshallow in git 1.8.3+ From 999deb36e9c398060952e490c09132335a717161 Mon Sep 17 00:00:00 2001 From: Alexander Grund Date: Thu, 5 Aug 2021 16:32:19 +0200 Subject: [PATCH 8/9] Mock and catch warning message --- test/framework/filetools.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/framework/filetools.py b/test/framework/filetools.py index 722fbf54da..4ea4738881 100644 --- a/test/framework/filetools.py +++ b/test/framework/filetools.py @@ -2589,7 +2589,10 @@ def run_check(): # use a tag that clashes with a branch name and make sure this is handled correctly git_config['tag'] = 'tag_for_tests' - res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + with self.mocked_stdout_stderr(): + res = ft.get_source_tarball_from_git('test.tar.gz', target_dir, git_config) + stderr = self.get_stderr() + self.assertIn('Tag tag_for_tests was not downloaded in the first try', stderr) self.assertEqual(res, test_file) self.assertTrue(os.path.isfile(test_file)) # Check that we indeed downloaded the tag and not the branch From 6a831521af6be42d103acdf157421d6d680fe502 Mon Sep 17 00:00:00 2001 From: Kenneth Hoste Date: Wed, 1 Sep 2021 15:48:31 +0200 Subject: [PATCH 9/9] trivial code style tweaking in get_source_tarball_from_git --- easybuild/tools/filetools.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/easybuild/tools/filetools.py b/easybuild/tools/filetools.py index 983abdcb76..635e897ee9 100644 --- a/easybuild/tools/filetools.py +++ b/easybuild/tools/filetools.py @@ -2481,7 +2481,8 @@ def get_source_tarball_from_git(filename, targetdir, git_config): if recursive: clone_cmd.append('--recursive') else: - clone_cmd.append('--no-checkout') # We do that manually below + # checkout is done separately below for specific commits + clone_cmd.append('--no-checkout') clone_cmd.append('%s/%s.git' % (url, repo_name)) @@ -2496,6 +2497,7 @@ def get_source_tarball_from_git(filename, targetdir, git_config): checkout_cmd.extend(['&&', 'git', 'submodule', 'update', '--init', '--recursive']) run.run_cmd(' '.join(checkout_cmd), log_all=True, simple=True, regexp=False, path=repo_name) + elif not build_option('extended_dry_run'): # If we wanted to get a tag make sure we actually got a tag and not a branch with the same name # This doesn't make sense in dry-run mode as we don't have anything to check @@ -2507,10 +2509,13 @@ def get_source_tarball_from_git(filename, targetdir, git_config): ' with the same name. You might want to alert the maintainers of %s about that issue.', tag, url, repo_name, repo_name) cmds = [] + if not keep_git_dir: - # Make the repo unshallow, same as git fetch --unshallow in git 1.8.3+ - # The first fetch seemingly does nothing, no idea why. + # make the repo unshallow first; + # this is equivalent with 'git fetch -unshallow' in Git 1.8.3+ + # (first fetch seems to do nothing, unclear why) cmds.append('git fetch --depth=2147483647 && git fetch --depth=2147483647') + cmds.append('git checkout refs/tags/' + tag) # Clean all untracked files, e.g. from left-over submodules cmds.append('git clean --force -d -x')