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']) 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) > + 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. > 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. > 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
