Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clone: Fetch the upstream remote specified in debian/upstream/metadata #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions gbp/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ class GbpOptionParser(OptionParser):
'track': 'True',
'track-missing': 'False',
'upstream-branch': 'upstream',
'upstream-remote': 'upstream',
'upstream-tag': 'upstream/%(version)s',
'upstream-tree': 'TAG',
'upstream-vcs-tag': '',
Expand All @@ -201,6 +202,11 @@ class GbpOptionParser(OptionParser):
'upstream-tree':
"Where to generate the upstream tarball from "
"(tag or branch), default is '%(upstream-tree)s'",
'upstream-remote':
"If debian/upstream/metadata specifies an upstream git repository, "
"the remote name to fetch it into, default is "
"'%(upstream-remote)s'. Set to an empty string to disable cloning "
"upstream",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of Set to an empty string to disable cloning "upstream" it'd rather do a default is ... as with most of the other options.

'pq-from':
"How to find the patch queue base. DEBIAN or TAG, "
"the default is '%(pq-from)s'",
Expand Down
26 changes: 26 additions & 0 deletions gbp/scripts/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ def build_parser(name):

branch_group.add_option("--all", action="store_true", dest="all", default=False,
help="track all branches, not only debian and upstream")
branch_group.add_config_file_option(option_name="upstream-remote", dest="upstream_remote")
branch_group.add_config_file_option(option_name="upstream-branch", dest="upstream_branch")
branch_group.add_config_file_option(option_name="debian-branch", dest="debian_branch")
branch_group.add_boolean_config_file_option(option_name="pristine-tar", dest="pristine_tar")
Expand Down Expand Up @@ -208,6 +209,31 @@ def main(argv):

repo_setup.set_user_name_and_email(options.repo_user, options.repo_email, repo)

if options.upstream_remote:
try:
with open(os.path.join(clone_to, 'debian', 'upstream', 'metadata')) as f:
import yaml

y = yaml.load(f, Loader=yaml.CSafeLoader)
try:
upstream_repo = y['Repository']

try:
# check if it exists as a git repo
repo.fetch(repo=upstream_repo)

repo.add_remote_repo(options.upstream_remote,
upstream_repo,
tags=True)
except GitRepositoryError:
gbp.log.warn(
'Unable to fetch upstream repository %s, '
'ignoring.' % upstream_repo)
except KeyError:
pass
except FileNotFoundError:
pass
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could that go into a function? Also we don't have a way to fail cloning if no upstream remote exists or when cloning fails. I'd expect things to fail if that feature is enabled otherwise things become hard to use in scripts. Maybe we need a separate option to enable adding upstream with [yes|no|auto] (the later meaning add upstream if metadata exists)?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thing is we can't know from debian/upstream/metadata whether it is a git repository, a Mercurial one, SVN or anything else. So here I just try to clone it and don't really further interpret the failure since it could be "expected". Could log a debug message?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's do heuristics then: So for --clone-upstream=auto :

  • if it's salsa: git
  • if it ends in .git: git
  • if it's gitlab.com or github.com: git

if cloning fails we give an error. If it does not match the pattern ignore the error (and print a warning). We could go further and make this configurable (similar to what we have for gbp create-remote-repo). For --clone-upstream=yes` we always try to clone and fail if cloning fails.

And plesse add a code comment that points to the bug that says we need to be clearer on metadata (i think i saw this flying by)


if postclone:
Hook('Postclone', options.postclone,
extra_env={'GBP_GIT_DIR': repo.git_dir},
Expand Down
8 changes: 7 additions & 1 deletion tests/component/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def check_tags(cls, repo, tags):

@classmethod
def _check_repo_state(cls, repo, current_branch, branches, files=None,
dirs=None, tags=None, clean=True):
dirs=None, tags=None, clean=True, remotes=[]):
"""
Check that repository is clean and given branches, tags, files
and dirs exist
Expand All @@ -191,6 +191,12 @@ def _check_repo_state(cls, repo, current_branch, branches, files=None,
local_branches)
eq_(set(local_branches), set(branches), assert_msg)

if remotes:
repo_remotes = repo.get_remotes().keys()
assert_msg = "Remotes: expected %s, found %s" % (remotes,
repo_remotes)
eq_(set(repo_remotes), set(remotes), assert_msg)

if files is not None or dirs is not None:
# Get files of the working copy recursively
local_f = set()
Expand Down
12 changes: 12 additions & 0 deletions tests/component/deb/test_clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,15 @@ def test_clone_github(self):
self.assertEquals(ret, 0)
cloned = ComponentTestGitRepository(dest)
self._check_repo_state(cloned, 'master', ['master'])

@skipUnless(os.getenv("GBP_NETWORK_TESTS"), "network tests disabled")
def test_clone_upstream_remote(self):
"""Test that cloning from github urls works"""
dest = os.path.join(self._tmpdir,
'cloned_repo')
ret = clone(['arg0', "vcsgit:tepl", dest])
self.assertEquals(ret, 0)
cloned = ComponentTestGitRepository(dest)
self._check_repo_state(cloned, 'debian/master',
['debian/master', 'upstream/latest', 'pristine-tar'],
remotes=['origin', 'upstream'])