#1688 rpmbuild: don't checkout master when not requested
Merged 4 years ago by msuchy. Opened 4 years ago by praiskup.

@@ -301,8 +301,8 @@

          else:

              raise e

  

-     if not committish:

-         committish = "master"

- 

-     checkout_cmd = ['git', 'checkout', committish]

-     run_cmd(checkout_cmd, cwd=repo_path)

+     if committish:

+         # Do the checkout only if explicitly requested, otherwise build against

+         # the default branch.

+         checkout_cmd = ['git', 'checkout', committish]

+         run_cmd(checkout_cmd, cwd=repo_path)

@@ -7,9 +7,11 @@

  import tempfile

  

  import configparser

+ import pytest

  

  from copr_distgit_client import check_output

  from copr_rpmbuild.providers.distgit import DistGitProvider

+ from copr_rpmbuild.helpers import git_clone_and_checkout

  

  try:

      from unittest import mock
@@ -86,3 +88,16 @@

          dgp.produce_srpm()

          assert os.path.exists(os.path.join(self.outdir, "obtain-sources",

                                             "origin", "datafile"))

+ 

+ 

+ @pytest.mark.parametrize('committish', ["main", None, ""])

+ @mock.patch("copr_rpmbuild.helpers.run_cmd")

+ def test_with_without_committish(run_cmd, committish):

+     git_clone_and_checkout("clone_url", committish, "/dir")

+     expected = []

+     expected += [mock.call(['git', 'clone', 'clone_url', '/dir', '--depth',

+                             '500', '--no-single-branch'])]

+     if committish:

+         expected += [mock.call(['git', 'checkout', 'main'], cwd='/dir')]

+ 

+     assert expected == run_cmd.call_args_list

When users don't specify the committish string explicitly we can simply
avoid the redundant 'git checkout' step. We'll keep the using the
branch that is checked-out by default by git (be that rawhide, main or
master, depending on concrete project).

Fixes: #1687

Build succeeded.

I agree. scm and distgit both should use the default branch (whatever it may be) if not explicitly specified, yes.

please take a look, this imo deserves copr-rpmbuild release

Not sure what to look at, the code seems reasonable.

As nit-pick, the test and implementation use the very same if conditional logic, which I'd recommend against, since it might hide a problem in the logic if it ever happens. Since the expected command hardcodes "main", I'd change the conditional in test to if committish == "main".

rebased onto de8cffb

4 years ago

Pull-Request has been merged by msuchy

4 years ago

Build succeeded.