A common source of (minor) frustration for non-packager-savvy people trying to clone package trees is:
$ fedpkg clone foobar Cloning into 'foobar'... Permission denied (publickey). fatal: Could not read from remote repository.
Please make sure you have the correct access rights and the repository exists. Could not execute clone: Command '['git', 'clone', 'ssh://you@pkgs.fedoraproject.org/foobar']' returned non-zero exit status 128 ERROR:rpkg:Could not execute clone: Command '['git', 'clone', 'ssh://you@pkgs.fedoraproject.org/foobar']' returned non-zero exit status 128
This is confusing for newcomers (it is not obvious they should have used the --anonymous/-a option), and it is unneeded: authentication is only really needed for pushing.
This change makes the --anonymous/-a options completely unnecessary: - clones are now always done anonymously - pushes now always require authentication
Using the options now prints a warning that they are unneeded, but keeps them to avoid breaking existing scripts. The API parameters are also kept to preserve compatibility with pyrpkg-based tools, but they are now completely unused. --- src/pyrpkg/__init__.py | 53 ++++++++++++++++++++++++++++++-------------------- src/pyrpkg/cli.py | 15 +++++++++----- 2 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/src/pyrpkg/__init__.py b/src/pyrpkg/__init__.py index 425ba5f..8437c26 100644 --- a/src/pyrpkg/__init__.py +++ b/src/pyrpkg/__init__.py @@ -1014,7 +1014,7 @@ class Commands(object): self._run_command(cmd, cwd=self.path) return
- def clone(self, module, path=None, branch=None, bare_dir=None, anon=False): + def clone(self, module, path=None, branch=None, bare_dir=None, anon=None): """Clone a repo, optionally check out a specific branch.
module is the name of the module to clone @@ -1026,19 +1026,22 @@ class Commands(object): bare_dir is the name of a directory to make a bare clone to, if this is a bare clone. None otherwise.
- anon is whether or not to clone anonymously + anon is now unused, but kept for compatibility with older pyrpkg-based + tools.
Logs the output and returns nothing.
"""
+ if anon is not None: + self.log.warning("The `anon` argument is now obsolete and " + "unneeded") + if not path: path = self.path # construct the git url - if anon: - giturl = self.anongiturl % {'module': module} - else: - giturl = self.gitbaseurl % {'user': self.user, 'module': module} + anon_giturl = self.anongiturl % {'module': module} + authed_giturl = self.gitbaseurl % {'user': self.user, 'module': module}
# Create the command cmd = ['git', 'clone'] @@ -1049,14 +1052,14 @@ class Commands(object): raise rpkgError('Cannot combine bare cloning with a branch') elif branch: # For now we have to use switch branch - self.log.debug('Checking out a specific branch %s' % giturl) - cmd.extend(['-b', branch, giturl]) + self.log.debug('Checking out a specific branch %s' % anon_giturl) + cmd.extend(['-b', branch, anon_giturl]) elif bare_dir: - self.log.debug('Cloning %s bare' % giturl) - cmd.extend(['--bare', giturl, bare_dir]) + self.log.debug('Cloning %s bare' % anon_giturl) + cmd.extend(['--bare', anon_giturl, bare_dir]) else: - self.log.debug('Cloning %s' % giturl) - cmd.extend([giturl]) + self.log.debug('Cloning %s' % anon_giturl) + cmd.extend([anon_giturl])
if not bare_dir: # --bare and --origin are incompatible @@ -1064,27 +1067,33 @@ class Commands(object):
self._run_command(cmd, cwd=path)
+ repo = git.Repo(os.path.join(path, module)) + repo.remotes["origin"].config_writer.set_value("pushurl", authed_giturl) + return
- def clone_with_dirs(self, module, anon=False): + def clone_with_dirs(self, module, anon=None): """Clone a repo old style with subdirs for each branch.
module is the name of the module to clone
- gitargs is an option list of arguments to git clone + anon is now unused, but kept for compatibility with older pyrpkg-based + tools.
"""
+ if anon is not None: + self.log.warning("The `anon` argument is now obsolete and " + "unneeded") + # Get the full path of, and git object for, our directory of branches top_path = os.path.join(self.path, module) top_git = git.Git(top_path) repo_path = os.path.join(top_path, 'rpkg.git')
- # construct the git url - if anon: - giturl = self.anongiturl % {'module': module} - else: - giturl = self.gitbaseurl % {'user': self.user, 'module': module} + # construct the git urls + anon_giturl = self.anongiturl % {'module': module} + authed_giturl = self.gitbaseurl % {'user': self.user, 'module': module}
# Create our new top directory try: @@ -1095,7 +1104,7 @@ class Commands(object):
# Create a bare clone first. This gives us a good list of branches try: - self.clone(module, top_path, bare_dir=repo_path, anon=anon) + self.clone(module, top_path, bare_dir=repo_path) except Exception, e: # Clean out our directory shutil.rmtree(top_path) @@ -1117,7 +1126,9 @@ class Commands(object): branch_path = os.path.join(top_path, branch) branch_git = git.Git(branch_path) branch_git.config("--replace-all", - "remote.%s.url" % self.branch_remote, giturl) + "remote.%s.url" % self.branch_remote, anon_giturl) + branch_git.config("--replace-all", + "remote.%s.pushurl" % self.branch_remote, authed_giturl) except (git.GitCommandError, OSError), e: raise rpkgError('Could not locally clone %s from %s: %s' % (branch, repo_path, e)) diff --git a/src/pyrpkg/cli.py b/src/pyrpkg/cli.py index 1ed8862..709d183 100755 --- a/src/pyrpkg/cli.py +++ b/src/pyrpkg/cli.py @@ -347,7 +347,8 @@ defined, packages will be built sequentially.""" % # allow to clone without needing a account on the scm server clone_parser.add_argument('--anonymous', '-a', action = 'store_true', - help = 'Check out a module anonymously') + help = 'Obsolete, kept for compatibility ' + 'only') # store the module to be cloned clone_parser.add_argument('module', nargs = 1, help = 'Name of the module to clone') @@ -978,11 +979,9 @@ defined, packages will be built sequentially.""" %
def clone(self): if self.args.branches: - self.cmd.clone_with_dirs(self.args.module[0], - anon=self.args.anonymous) + self.cmd.clone_with_dirs(self.args.module[0]) else: - self.cmd.clone(self.args.module[0], branch=self.args.branch, - anon=self.args.anonymous) + self.cmd.clone(self.args.module[0], branch=self.args.branch)
def commit(self): if self.args.clog: @@ -1341,6 +1340,12 @@ Tasks still running. You can continue to watch with the '%s watch-task' command.
# Parse the args self.args = self.parser.parse_args() + + if "anonymous" in self.args and self.args.anonymous: + # Logging is not setup yet + sys.stderr.write('WARNING: The --anonymous/-a option is now ' + 'obsolete and unneeded\n') + if self.args.user: self.user = self.args.user else: