Thanks for the review! All done: On Mon, Nov 28, 2016 at 8:37 AM, Guido Günther <[email protected]> wrote:
> Hi Michael, > thanks for working on this. > > On Sat, Nov 26, 2016 at 07:17:59PM +0100, Michael Stapelberg wrote: > > On Fri, Nov 25, 2016 at 2:08 PM, Guido Günther <[email protected]> wrote: > > > > > control: forecmerge 679121 -1 > > > > > > > This should have read “forcemerge”, I assume. I’ll let you correct that > and > > just reply to this bug for now. > > No worries, the bts told me too so it's merged already. > > > > control: tags 679121 -patch > > > > > > Hi Michael, > > > thanks for the patch! I think we're heading in the right direction. > > > > > > On Thu, Nov 24, 2016 at 12:26:19PM +0100, Michael Stapelberg wrote: > > > > Package: git-buildpackage > > > > Version: 0.8.6 > > > > Severity: wishlist > > > > Tags: patch > > > > > > > > I realize that https://bugs.debian.org/679121 is similar. In case > you > > > > prefer to close this issue in favor of #679121, please update #679121 > > > > with a clear decision as to how honouring DEBFULLNAME and DEBEMAIL in > > > > git-buildpackage should be implemented, and I’ll be happy to follow > up. > > > > > > > > Until that’s worked out, I’d like to propose a slightly different > > > > approach which I have been using for years: at clone-time, I set > > > > user.email to my debian email address. > > > > > > The main reason 679121 is still open is that it wasn't clear to me > where > > > exactly gbp should use DEBEMAIL/DEBFULLNAME and where not but what you > > > propose makes sense: use it everywhere gbp creates repos to set up sane > > > defaults: > > > > > > * gbp clone > > > * gbp import-dsc > > > * gbp ipmort-srpm > > > > > > But we need to make it configurable and add a test to make sure we > don't > > > break it in the future (e.g. in tests/component/deb/test_clone.py). > > > > > > > Makes sense to me. > > > > I’ve updated the patch (see attachment) to cover gbp clone, gbp > import-dsc > > and gbp import-srpm. I have also added a test in test_clone.py, as you > > suggested. > > > > With regards to configuration, could you please tell me how you’d like > the > > option to be called? You’re more familiar with git-buildpackage and hence > > can give a recommendation for a consistent option name :). > > I don't have a great suggestion for that one but given that we might > have several ways to init the _user_ for the new _repo_ in the future > something like: > > --repo-user=DEBIAN : use debemail > --repo-user=GIT : let git figure out what to do > > make sense to me. That would allow for other setup modes > like--repo-user=rpm or --repo-user=matching (DEB* for the debian tools > and something else for the rpm tools) in the future. The uppercase > allows to distinguish this from > > --repo-user="Foobar" > > easily in case we want to allow to set an explicit user name in the future. > S.th. like > > > parser.add_config_file_option(…, choices=['DEBIAN', 'GIT']) > Done. > > Does this make sense? > > > Let me know if anything else should be changed in the patch, or feel free > > to apply it and change it yourself as you see fit. > > Pleae see below. > > > > > > > > Cheers, > > > -- Guido > > > > > > > > > > > Please consider merging the attached patch. Thank you! > > > > > > > > -- System Information: > > > > Debian Release: stretch/sid > > > > APT prefers testing > > > > APT policy: (990, 'testing'), (500, 'unstable') > > > > Architecture: amd64 (x86_64) > > > > Foreign Architectures: i386, armel, mipsel > > > > > > > > Kernel: Linux 4.6.0-1-amd64 (SMP w/8 CPU cores) > > > > Locale: LANG=de_DE.UTF-8, LC_CTYPE=de_DE.UTF-8 (charmap=UTF-8) > > > > Shell: /bin/sh linked to /bin/dash > > > > Init: systemd (via /run/systemd/system) > > > > > > > > Versions of packages git-buildpackage depends on: > > > > ii devscripts 2.16.7 > > > > ii git 1:2.9.3-1 > > > > ii man-db 2.7.5-1 > > > > ii python-dateutil 2.4.2-1 > > > > ii python-pkg-resources 25.2.0-1 > > > > ii python-six 1.10.0-3 > > > > pn python:any <none> > > > > > > > > Versions of packages git-buildpackage recommends: > > > > ii cowbuilder 0.80 > > > > ii pbuilder 0.225.2 > > > > ii pristine-tar 1.34 > > > > ii python-requests 2.10.0-2 > > > > ii sbuild 0.71.0-2 > > > > > > > > Versions of packages git-buildpackage suggests: > > > > pn python-notify <none> > > > > ii sudo 1.8.17p1-2 > > > > ii unzip 6.0-20 > > > > > > > > -- no debconf information > > > > > > > >From 9d4f3ae0b3a783e8c96f1e83c9c2192c4fe0b56c Mon Sep 17 00:00:00 > 2001 > > > > From: Michael Stapelberg <[email protected]> > > > > Date: Thu, 24 Nov 2016 12:17:50 +0100 > > > > Subject: [PATCH] gbp clone: configure user.email from DEBEMAIL > > > > > > > > --- > > > > gbp/git/repository.py | 10 ++++++++++ > > > > gbp/scripts/clone.py | 3 +++ > > > > 2 files changed, 13 insertions(+) > > > > > > > > diff --git a/gbp/git/repository.py b/gbp/git/repository.py > > > > index 2f1b71b..d30ec07 100644 > > > > --- a/gbp/git/repository.py > > > > +++ b/gbp/git/repository.py > > > > @@ -1063,6 +1063,16 @@ class GitRepository(object): > > > > raise KeyError > > > > return value[0][:-1] # first line with \n ending removed > > > > > > > > + def set_user_email(self, email): > > > > + """ > > > > + Sets the email address to use for git commits. > > > > + > > > > + @param email: email address to use > > > > + """ > > > > + args = GitArgs() > > > > + args.add('user.email', email) > > > > + self._git_command("config", args.args) > > > > + > > > > def get_author_info(self): > > > > """ > > > > Determine a sane values for author name and author email > from > > > git's > > > > diff --git a/gbp/scripts/clone.py b/gbp/scripts/clone.py > > > > index 63b1468..2313acf 100755 > > > > --- a/gbp/scripts/clone.py > > > > +++ b/gbp/scripts/clone.py > > > > @@ -128,6 +128,9 @@ def main(argv): > > > > > > > > repo.set_branch(options.debian_branch) > > > > > > > > + if os.getenv('DEBEMAIL'): > > > > + repo.set_user_email(os.getenv('DEBEMAIL')) > > > > + > > > > if postclone: > > > > Hook('Postclone', options.postclone, > > > > extra_env={'GBP_GIT_DIR': repo.git_dir}, > > > > -- > > > > 2.9.3 > > > > > > > > > > > > > > > > -- > > Best regards, > > Michael > > > From 71a55422bd1b2506c34bebf6b36026152583f0f4 Mon Sep 17 00:00:00 2001 > > From: Michael Stapelberg <[email protected]> > > Date: Thu, 24 Nov 2016 12:17:50 +0100 > > Subject: [PATCH] gbp clone: configure user.email, user.name from > > DEBEMAIL/DEBFULLNAME > > > > --- > > gbp/git/repository.py | 20 ++++++++++++++++++++ > > gbp/scripts/clone.py | 6 ++++++ > > gbp/scripts/import_dsc.py | 6 ++++++ > > gbp/scripts/import_srpm.py | 6 ++++++ > > tests/component/deb/test_clone.py | 24 ++++++++++++++++++++++++ > > 5 files changed, 62 insertions(+) > > > > diff --git a/gbp/git/repository.py b/gbp/git/repository.py > > index 2f1b71b..b4c16e6 100644 > > --- a/gbp/git/repository.py > > +++ b/gbp/git/repository.py > > @@ -1063,6 +1063,26 @@ class GitRepository(object): > > raise KeyError > > return value[0][:-1] # first line with \n ending removed > > > > + def set_user_name(self, name): > > + """ > > + Sets the full name to use for git commits. > > + > > + @param name: full name to use > > + """ > > + args = GitArgs() > > + args.add('user.name', name) > > This can be written in one line: > > args = GitArgs('user.name', name) > Done. > > > > + self._git_command("config", args.args) > > + > > + def set_user_email(self, email): > > + """ > > + Sets the email address to use for git commits. > > + > > + @param email: email address to use > > + """ > > + args = GitArgs() > > + args.add('user.email', email) > > + self._git_command("config", args.args) > > + > > If these would get unit tests in > > tests/doctests/test_GitRepository.py > > that would be awesome. > Done. > > > > def get_author_info(self): > > """ > > Determine a sane values for author name and author email from > git's > > diff --git a/gbp/scripts/clone.py b/gbp/scripts/clone.py > > index 63b1468..14b8646 100755 > > --- a/gbp/scripts/clone.py > > +++ b/gbp/scripts/clone.py > > @@ -128,6 +128,12 @@ def main(argv): > > > > repo.set_branch(options.debian_branch) > > > > + if os.getenv('DEBFULLNAME'): > > + repo.set_user_name(os.getenv('DEBFULLNAME')) > > + > > + if os.getenv('DEBEMAIL'): > > + repo.set_user_email(os.getenv('DEBEMAIL')) > > + > > Ths should go into gbp/scripts/common/repo_setup.py and used in all the > tools to avoid duplication. > Done. > > > if postclone: > > Hook('Postclone', options.postclone, > > extra_env={'GBP_GIT_DIR': repo.git_dir}, > > diff --git a/gbp/scripts/import_dsc.py b/gbp/scripts/import_dsc.py > > index aa734e8..e1187ba 100644 > > --- a/gbp/scripts/import_dsc.py > > +++ b/gbp/scripts/import_dsc.py > > @@ -335,6 +335,12 @@ def main(argv): > > if repo.bare: > > disable_pristine_tar(options, "Bare repository") > > > > + if os.getenv('DEBFULLNAME'): > > + repo.set_user_name(os.getenv('DEBFULLNAME')) > > + > > + if os.getenv('DEBEMAIL'): > > + repo.set_user_email(os.getenv('DEBEMAIL')) > > + > > dirs['tmp'] = os.path.abspath(tempfile.mkdtemp(dir='..')) > > upstream = DebianUpstreamSource(src.tgz) > > upstream.unpack(dirs['tmp'], options.filters) > > diff --git a/gbp/scripts/import_srpm.py b/gbp/scripts/import_srpm.py > > index c4b3a48..ce0eb4b 100755 > > --- a/gbp/scripts/import_srpm.py > > +++ b/gbp/scripts/import_srpm.py > > @@ -264,6 +264,12 @@ def main(argv): > > repo = RpmGitRepository.create(spec.name) > > os.chdir(repo.path) > > > > + if os.getenv('DEBFULLNAME'): > > + repo.set_user_name(os.getenv('DEBFULLNAME')) > > + > > + if os.getenv('DEBEMAIL'): > > + repo.set_user_email(os.getenv('DEBEMAIL')) > > + > > if repo.bare: > > set_bare_repo_options(options) > > > > diff --git a/tests/component/deb/test_clone.py > b/tests/component/deb/test_clone.py > > index 0c3ba2c..61bf46e 100644 > > --- a/tests/component/deb/test_clone.py > > +++ b/tests/component/deb/test_clone.py > > @@ -72,3 +72,27 @@ class TestClone(ComponentTestBase): > > self._check_repo_state(cloned, 'master', ['master']) > > assert len(cloned.get_commits()) == 1 > > self.check_hook_vars('postclone', ["GBP_GIT_DIR"]) > > + > > + def test_clone_environ(self): > > + """Test that environment variables influence git > configuration""" > > + def _dsc(version): > > + return os.path.join(DEB_TEST_DATA_DIR, > > + 'dsc-native', > > + 'git-buildpackage_%s.dsc' % version) > > + > > + # Build up somethng we can clone from > > + dsc = _dsc('0.4.14') > > + os.environ['DEBFULLNAME'] = 'testing tester' > > + os.environ['DEBEMAIL'] = '[email protected]' > > + assert import_dsc(['arg0', dsc]) == 0 > > + repo = ComponentTestGitRepository('git-buildpackage') > > + self._check_repo_state(repo, 'master', ['master']) > > + assert len(repo.get_commits()) == 1 > > + > > + got = repo.get_config("user.email") > > + want = os.environ['DEBEMAIL'] > > + ok_(got == want, "unexpected git config user.email: got %s, > want %s" % (got, want)) > > + > > + got = repo.get_config("user.name") > > + want = os.environ['DEBFULLNAME'] > > + ok_(got == want, "unexpected git config user.name: got %s, > want %s" % (got, want)) > > -- > > 2.10.2 > > > > Thanks, > -- Guido > -- Best regards, Michael
From ed0894e183191fa9f55541aa69d6933c26b2fcc6 Mon Sep 17 00:00:00 2001 From: Michael Stapelberg <[email protected]> Date: Thu, 24 Nov 2016 12:17:50 +0100 Subject: [PATCH] gbp clone: configure user.email, user.name from DEBEMAIL/DEBFULLNAME --- gbp/config.py | 12 +++++++++++- gbp/git/repository.py | 18 ++++++++++++++++++ gbp/scripts/clone.py | 7 +++++++ gbp/scripts/import_dsc.py | 8 ++++++++ gbp/scripts/import_srpm.py | 8 ++++++++ tests/component/deb/test_clone.py | 24 ++++++++++++++++++++++++ tests/doctests/test_GitRepository.py | 13 +++++++++++++ 7 files changed, 89 insertions(+), 1 deletion(-) diff --git a/gbp/config.py b/gbp/config.py index 149a1bb..18a3afc 100644 --- a/gbp/config.py +++ b/gbp/config.py @@ -181,6 +181,8 @@ class GbpOptionParser(OptionParser): 'component': [], 'bare': 'True', 'urgency': 'medium', + 'repo-user': 'DEBIAN', + 'repo-email': 'DEBIAN', } help = { 'debian-branch': @@ -350,7 +352,15 @@ class GbpOptionParser(OptionParser): "wether to create a bare repository on the remote side. " "'Default is '%(bare)s'.", 'urgency': - "Set urgency level, default is '%(urgency)s'" + "Set urgency level, default is '%(urgency)s'", + 'repo-user': + "Set repo username from the DEBFULLNAME and DEBEMAIL " + "environment variables ('DEBIAN') or fallback to the " + "git configuration ('GIT'), default is '%(repo-user)s'", + 'repo-email': + "Set repo email from the DEBFULLNAME and DEBEMAIL " + "environment variables ('DEBIAN') or fallback to the " + "git configuration ('GIT'), default is '%(repo-email)s'" } short_opts = { diff --git a/gbp/git/repository.py b/gbp/git/repository.py index 2f1b71b..644fc7b 100644 --- a/gbp/git/repository.py +++ b/gbp/git/repository.py @@ -1063,6 +1063,24 @@ class GitRepository(object): raise KeyError return value[0][:-1] # first line with \n ending removed + def set_user_name(self, name): + """ + Sets the full name to use for git commits. + + @param name: full name to use + """ + args = GitArgs('user.name', name) + self._git_command("config", args.args) + + def set_user_email(self, email): + """ + Sets the email address to use for git commits. + + @param email: email address to use + """ + args = GitArgs('user.email', email) + self._git_command("config", args.args) + def get_author_info(self): """ Determine a sane values for author name and author email from git's diff --git a/gbp/scripts/clone.py b/gbp/scripts/clone.py index 63b1468..0afa54c 100755 --- a/gbp/scripts/clone.py +++ b/gbp/scripts/clone.py @@ -26,6 +26,7 @@ from gbp.deb.git import DebianGitRepository from gbp.git import (GitRepository, GitRepositoryError) from gbp.errors import GbpError from gbp.scripts.common import ExitCodes +from gbp.scripts.common import repo_setup from gbp.scripts.common.hook import Hook import gbp.log @@ -62,6 +63,10 @@ def build_parser(name): parser.add_config_file_option(option_name="color", dest="color", type='tristate') parser.add_config_file_option(option_name="color-scheme", dest="color_scheme") + parser.add_config_file_option(option_name="repo-user", dest="repo_user", + choices=['DEBIAN', 'GIT']) + parser.add_config_file_option(option_name="repo-email", dest="repo_email", + choices=['DEBIAN', 'GIT']) return parser @@ -128,6 +133,8 @@ def main(argv): repo.set_branch(options.debian_branch) + repo_setup.set_user_name_and_email(options.repo_user, options.repo_email, repo) + if postclone: Hook('Postclone', options.postclone, extra_env={'GBP_GIT_DIR': repo.git_dir}, diff --git a/gbp/scripts/import_dsc.py b/gbp/scripts/import_dsc.py index 6113032..300c236 100644 --- a/gbp/scripts/import_dsc.py +++ b/gbp/scripts/import_dsc.py @@ -35,6 +35,7 @@ from gbp.config import (GbpOptionParserDebian, GbpOptionGroup, no_upstream_branch_msg) from gbp.errors import GbpError from gbp.scripts.common import ExitCodes +from gbp.scripts.common import repo_setup import gbp.log @@ -269,6 +270,11 @@ def build_parser(name): dest="author_committer_date") import_group.add_boolean_config_file_option(option_name="allow-unauthenticated", dest="allow_unauthenticated") + + parser.add_config_file_option(option_name="repo-user", dest="repo_user", + choices=['DEBIAN', 'GIT']) + parser.add_config_file_option(option_name="repo-email", dest="repo_email", + choices=['DEBIAN', 'GIT']) return parser @@ -341,6 +347,8 @@ def main(argv): if repo.bare: disable_pristine_tar(options, "Bare repository") + repo_setup.set_user_name_and_email(options.repo_user, options.repo_email, repo) + dirs['tmp'] = os.path.abspath(tempfile.mkdtemp(dir='..')) upstream = DebianUpstreamSource(src.tgz) upstream.unpack(dirs['tmp'], options.filters) diff --git a/gbp/scripts/import_srpm.py b/gbp/scripts/import_srpm.py index f98a659..e483191 100755 --- a/gbp/scripts/import_srpm.py +++ b/gbp/scripts/import_srpm.py @@ -37,6 +37,7 @@ from gbp.config import (GbpOptionParserRpm, GbpOptionGroup, no_upstream_branch_msg) from gbp.errors import GbpError from gbp.scripts.common import ExitCodes +from gbp.scripts.common import repo_setup import gbp.log from gbp.pkg import parse_archive_filename @@ -186,6 +187,11 @@ def build_parser(name): dest="author_is_committer") import_group.add_config_file_option(option_name="packaging-dir", dest="packaging_dir") + + parser.add_config_file_option(option_name="repo-user", dest="repo_user", + choices=['DEBIAN', 'GIT']) + parser.add_config_file_option(option_name="repo-email", dest="repo_email", + choices=['DEBIAN', 'GIT']) return parser @@ -273,6 +279,8 @@ def main(argv): repo = RpmGitRepository.create(target) os.chdir(repo.path) + repo_setup.set_user_name_and_email(options.repo_user, options.repo_email, repo) + if repo.bare: set_bare_repo_options(options) diff --git a/tests/component/deb/test_clone.py b/tests/component/deb/test_clone.py index 0c3ba2c..61bf46e 100644 --- a/tests/component/deb/test_clone.py +++ b/tests/component/deb/test_clone.py @@ -72,3 +72,27 @@ class TestClone(ComponentTestBase): self._check_repo_state(cloned, 'master', ['master']) assert len(cloned.get_commits()) == 1 self.check_hook_vars('postclone', ["GBP_GIT_DIR"]) + + def test_clone_environ(self): + """Test that environment variables influence git configuration""" + def _dsc(version): + return os.path.join(DEB_TEST_DATA_DIR, + 'dsc-native', + 'git-buildpackage_%s.dsc' % version) + + # Build up somethng we can clone from + dsc = _dsc('0.4.14') + os.environ['DEBFULLNAME'] = 'testing tester' + os.environ['DEBEMAIL'] = '[email protected]' + assert import_dsc(['arg0', dsc]) == 0 + repo = ComponentTestGitRepository('git-buildpackage') + self._check_repo_state(repo, 'master', ['master']) + assert len(repo.get_commits()) == 1 + + got = repo.get_config("user.email") + want = os.environ['DEBEMAIL'] + ok_(got == want, "unexpected git config user.email: got %s, want %s" % (got, want)) + + got = repo.get_config("user.name") + want = os.environ['DEBFULLNAME'] + ok_(got == want, "unexpected git config user.name: got %s, want %s" % (got, want)) diff --git a/tests/doctests/test_GitRepository.py b/tests/doctests/test_GitRepository.py index bd7c005..a5d6c53 100644 --- a/tests/doctests/test_GitRepository.py +++ b/tests/doctests/test_GitRepository.py @@ -996,4 +996,17 @@ def test_cmd_has_feature(): True """ + +def test_set_user_name_and_email(): + r""" + Methods tested: + - L{gbp.git.GitRepository.set_user_name} + - L{gbp.git.GitRepository.set_user_email} + + >>> import gbp.git + >>> repo = gbp.git.GitRepository(dirs['repo']) + >>> repo.set_user_name("Michael Stapelberg") + >>> repo.set_user_email("[email protected]") + """ + # vim:et:ts=4:sw=4:et:sts=4:ai:set list listchars=tab\:»·,trail\:·: -- 2.10.2
