diff --git a/fdroidserver/build.py b/fdroidserver/build.py index 41df2c1c..e5ee9663 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -721,8 +721,7 @@ def build_local(app, build, vcs, build_dir, output_dir, log_dir, srclib_dir, ext bindir = os.path.join(root_dir, 'bin') if os.path.isdir(os.path.join(build_dir, '.git')): - import git - commit_id = common.get_head_commit_id(git.repo.Repo(build_dir)) + commit_id = str(common.get_head_commit_id(build_dir)) else: commit_id = build.commit diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 4a30bb6b..ce5dc195 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1286,12 +1286,29 @@ def write_status_json(output, pretty=False, name=None): rsync_status_file_to_repo(path, repo_subdir='status') -def get_head_commit_id(git_repo): - """Get git commit ID for HEAD as a str.""" +def get_head_commit_id(git_repo_dir): + """Get git commit ID for HEAD as a str. + + This only reads files, so it should be safe to use on untrusted + repos. It was created to avoid running the git executable, no + matter what. It uses a tiny subset of the git.Repo class to avoid + setting up the git executable. + + """ try: - return git_repo.head.commit.hexsha - except ValueError: - return "None" + if type(git_repo_dir) is git.Repo: + d = git_repo_dir.git_dir + else: + d = os.path.join(git_repo_dir, '.git') + repo = type( + 'Repo', + (object,), + {'common_dir': d, 'git_dir': d, 're_hexsha_only': git.Repo.re_hexsha_only}, + )() + return git.refs.symbolic.SymbolicReference.dereference_recursive(repo, 'HEAD') + except (FileNotFoundError, ValueError) as e: + msg = _("Cannot read {path}: {error}").format(path=os.getcwd(), error=str(e)) + logging.debug(msg) def setup_vcs(app): diff --git a/fdroidserver/import_subcommand.py b/fdroidserver/import_subcommand.py index b9f9a4c4..345c2891 100644 --- a/fdroidserver/import_subcommand.py +++ b/fdroidserver/import_subcommand.py @@ -342,7 +342,7 @@ def main(): app.AutoUpdateMode = 'Version' app.UpdateCheckMode = 'Tags' - build.commit = common.get_head_commit_id(git_repo) + build.commit = common.get_head_commit_id(tmp_importer_dir) # Extract some information... paths = get_all_gradle_and_manifests(tmp_importer_dir) diff --git a/fdroidserver/metadata.py b/fdroidserver/metadata.py index f5fcffac..47fbdca7 100644 --- a/fdroidserver/metadata.py +++ b/fdroidserver/metadata.py @@ -18,7 +18,6 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -import git from pathlib import Path import math import platform @@ -658,14 +657,12 @@ def parse_metadata(metadatapath): build_dir = common.get_build_dir(app) metadata_in_repo = build_dir / '.fdroid.yml' if metadata_in_repo.is_file(): - try: - commit_id = common.get_head_commit_id(git.Repo(build_dir)) + commit_id = common.get_head_commit_id(build_dir) + if commit_id is not None: logging.debug( _('Including metadata from %s@%s') % (metadata_in_repo, commit_id) ) - # See https://github.com/PyCQA/pylint/issues/2856 . - # pylint: disable-next=no-member - except git.exc.InvalidGitRepositoryError: + else: logging.debug( _('Including metadata from {path}').format(path=metadata_in_repo) ) diff --git a/tests/test_common.py b/tests/test_common.py index 522367a4..f6cdb0cb 100755 --- a/tests/test_common.py +++ b/tests/test_common.py @@ -3470,3 +3470,53 @@ class UnsafePermissionsTest(SetUpTearDownMixin, unittest.TestCase): fdroidserver.common.read_config() self.assertTrue('unsafe' not in lw.output[0]) self.assertEqual(1, len(lw.output)) + + +class GetHeadCommitIdTest(unittest.TestCase): + """Test and compare two methods of getting the commit ID.""" + + def setUp(self): + self._td = mkdtemp() + self.testdir = self._td.name + os.chdir(self.testdir) + logging.getLogger('git.cmd').setLevel(logging.INFO) + + def tearDown(self): + os.chdir(basedir) + self._td.cleanup() + + @unittest.skipUnless((basedir.parent / '.git').exists(), 'Needs a working git repo') + def test_get_head_commit_id_compare(self): + """Run on this git repo to get some real world noise in there.""" + git_dir = basedir.parent + self.assertIsNotNone(fdroidserver.common.get_head_commit_id(git_dir)) + + def test_get_head_commit_id_error_bare_repo(self): + """Error because it is an empty, bare git repo.""" + git_repo = git.Repo.init(self.testdir) + self.assertIsNone(fdroidserver.common.get_head_commit_id(git_repo)) + + def test_get_head_commit_id_error_no_repo(self): + """Error because there is no .git/ dir.""" + with self.assertLogs('root', level=logging.DEBUG): + self.assertIsNone(fdroidserver.common.get_head_commit_id(self.testdir)) + + def test_get_head_commit_id_detached_and_branch(self): + """Fetching commit ID must work from detached HEADs and branches.""" + git_repo = git.Repo.init(self.testdir) + Path('README').write_text('this is just a test') + git_repo.git.add(all=True) + git_repo.index.commit("add README") + Path('LICENSE').write_text('free!') + git_repo.git.add(all=True) + git_repo.index.commit("add LICENSE") + self.assertIsNotNone(fdroidserver.common.get_head_commit_id(git_repo)) + # detached HEAD + git_repo.git.checkout('HEAD^') + self.assertIsNotNone(fdroidserver.common.get_head_commit_id(git_repo)) + # on a branch with a new commits + git_repo.git.checkout('test', b=True) + Path('foo.py').write_text('print("code!")') + git_repo.git.add(all=True) + git_repo.index.commit("add code") + self.assertIsNotNone(fdroidserver.common.get_head_commit_id(git_repo))