[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#854358: unblock: dgit/3.10



Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock

Please unblock package dgit

unblock dgit/3.10


I would like fix some bugs by providing a new dgit in stretch.  I have
not yet uploaded this package to sid, in case you dislike some of my
changes and want an even more minimal update.  (If that is the case, I
can strip them out and retest, since I have them as individual git
commits.)

Below you can find a summary of the changes, and references to enable
you to review the proposed update in detail.


I attach a `git diff' (which is going to be identical to the debdiff
of the package I will upload if you approve, modulo changelog
timestamp and any updates requested by you).

But you may prefer to review the changes as very fine-grained git
commits.  These are available in my personal git repository at:
  git://git.chiark.greenend.org.uk/~ianmdlvl/dgit.git 
as the changes 9b2d1c414982a096...e800a2be9f43a049 (aka the changes
from current `dgit fetch stretch' to my personal `stable'.)
So for example, you could:
  git clone git://git.chiark.greenend.org.uk/~ianmdlvl/dgit.git
  cd dgit
  git-log --reverse --stat -p 9b2d1c414982a096...e800a2be9f43a049


The updates in my proposed 3.10 correspond precisely to the bugs
marked "pending" in the BTS.  I will summarise them for you:

There are three fixes for bugs marked "important":

 #852661  dgit: fails to fetch when Maintainer has `,` in name

     This can prevent dgit from being used, at all, on some packages
     currently in Debian.

 #853085  ud private tree needs some git config copying from user's

     This can cause dgit to produce commits containing unintended (but
     usually not *entirely* wrong) authorship information.

 #853093  git `3.0 (quilt)' import can generate commits with extra
          blank line

     This causes dgit to generate suboptimal commits.  (The extra
     blank line is helpfully ignored by most git tools.)

There are two further bugfixes:

 #853022  dgit fetch can fail on detached head

     The fix to this is simple and obviously does not disturb existing
     functionality.  This seems likely to be a thing that many users
     will do, and the error message is quite unenlightening.

 #853125  fix typos in dgit documentation

     These seem to me to be covered by the freeze policy.

Additionally, the changes include changes to the autopkgtest test
suite, to add regression tests for most of the bugfixes.

The new tests do not pose a stability or FTBFS risk because the tests
are not run during package build.  The test suite changes are the
changes to tests/ and to debian/tests/control.


As part of my usual pre-releaase-preparation I have run this through
the test suite both using the ad-hoc in-tree arrangements, and
formally via autopkgtest.


Thanks four your attention.

Regards,
Ian.
 debian/changelog               | 22 ++++++++++++++++++++++
 debian/tests/control           |  2 +-
 dgit                           | 22 ++++++++++++++++++++--
 dgit-nmu-simple.7.pod          |  4 ++--
 dgit-user.7.pod                |  2 +-
 tests/lib-import-chk           | 13 +++++++++++++
 tests/tests/import-maintmangle | 41 +++++++++++++++++++++++++++++++++++++++++
 tests/tests/quilt-useremail    | 27 +++++++++++++++++++++++++++
 8 files changed, 127 insertions(+), 6 deletions(-)

diff --git a/debian/changelog b/debian/changelog
index 5b674604..24a1bf0d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,25 @@
+dgit (3.10) unstable; urgency=medium
+
+  Bugfixes:
+  * dgit: Copy several user.* settings from main tree git local config
+    to dgit private workarea.  Closes:#853085.
+  * dgit: Strip initial newline from Changes line from dpkg-parsechangelog
+    so as to avoid blank line in commit messages.  Closes:#853093.
+  * dgit: Do not fail when run with detached HEAD.  Closes:#853022.
+  * dgit: Be much better about commas in maintainer changelog names.
+    Closes:#852661.
+
+  Test suite:
+  * quilt-useremail: New test for user config copying (#853085).
+  * lib-import-chk: Test that commits have smae authorship as appears in
+    the changelog.  (Or, at least, the same authorship set.)
+  * import-maintmangle: New test for changelog Maintainer mangling.
+
+  Documentation:
+  * Fix typos.  Closes:#853125.  [Nicholas D Steeves]
+
+ -- Ian Jackson <ijackson@chiark.greenend.org.uk>  Sun, 05 Feb 2017 20:50:34 +0000
+
 dgit (3.9) unstable; urgency=medium
 
   Improvements:
diff --git a/debian/tests/control b/debian/tests/control
index ea79c22d..0df610e7 100644
--- a/debian/tests/control
+++ b/debian/tests/control
@@ -29,7 +29,7 @@ Tests-Directory: tests/tests
 Depends: dgit, dgit-infrastructure, devscripts, debhelper (>=8), fakeroot, build-essential
 Restrictions: x-dgit-git-only
 
-Tests: absurd-gitapply badcommit-rewrite build-modes build-modes-asplit build-modes-gbp-asplit clone-clogsigpipe clone-gitnosuite clone-nogit debpolicy-dbretry debpolicy-newreject debpolicy-quilt-gbp defdistro-rpush defdistro-setup distropatches-reject drs-clone-nogit drs-push-masterupdate drs-push-rejects dsd-clone-nogit dsd-divert fetch-localgitonly fetch-somegit-notlast gbp-orig gitconfig import-dsc import-native import-nonnative import-tarbomb inarchivecopy mismatches-contents mismatches-dscchanges multisuite newtag-clone-nogit oldnewtagalt oldtag-clone-nogit orig-include-exclude orig-include-exclude-chkquery overwrite-chkclog overwrite-junk overwrite-splitbrains overwrite-version protocol-compat push-buildproductsdir push-newpackage push-nextdgit quilt quilt-gbp quilt-gbp-build-modes quilt-singlepatch quilt-splitbrains rpush tag-updates test-list-uptodate trustingpolicy-replay unrepresentable version-opt
+Tests: absurd-gitapply badcommit-rewrite build-modes build-modes-asplit build-modes-gbp-asplit clone-clogsigpipe clone-gitnosuite clone-nogit debpolicy-dbretry debpolicy-newreject debpolicy-quilt-gbp defdistro-rpush defdistro-setup distropatches-reject drs-clone-nogit drs-push-masterupdate drs-push-rejects dsd-clone-nogit dsd-divert fetch-localgitonly fetch-somegit-notlast gbp-orig gitconfig import-dsc import-maintmangle import-native import-nonnative import-tarbomb inarchivecopy mismatches-contents mismatches-dscchanges multisuite newtag-clone-nogit oldnewtagalt oldtag-clone-nogit orig-include-exclude orig-include-exclude-chkquery overwrite-chkclog overwrite-junk overwrite-splitbrains overwrite-version protocol-compat push-buildproductsdir push-newpackage push-nextdgit quilt quilt-gbp quilt-gbp-build-modes quilt-singlepatch quilt-splitbrains quilt-useremail rpush tag-updates test-list-uptodate trustingpolicy-replay unrepresentable version-opt
 Tests-Directory: tests/tests
 Depends: dgit, dgit-infrastructure, devscripts, debhelper (>=8), fakeroot, build-essential
 
diff --git a/dgit b/dgit
index 9cdf96b5..6b1201e7 100755
--- a/dgit
+++ b/dgit
@@ -1699,6 +1699,11 @@ sub prep_ud (;$) {
 sub mktree_in_ud_here () {
     runcmd qw(git init -q);
     runcmd qw(git config gc.auto 0);
+    foreach my $copy (qw(user.email user.name user.useConfigOnly)) {
+	my $v = $gitcfgs{local}{$copy};
+	next unless $v;
+	runcmd qw(git config), $copy, $_ foreach @$v;
+    }
     rmtree('.git/objects');
     symlink '../../../../objects','.git/objects' or die $!;
     setup_gitattrs(1);
@@ -1990,7 +1995,14 @@ sub make_commit_text ($) {
 sub clogp_authline ($) {
     my ($clogp) = @_;
     my $author = getfield $clogp, 'Maintainer';
-    $author =~ s#,.*##ms;
+    if ($author =~ m/^[^"\@]+\,/) {
+	# single entry Maintainer field with unquoted comma
+	$author = ($& =~ y/,//rd).$'; # strip the comma
+    }
+    # git wants a single author; any remaining commas in $author
+    # are by now preceded by @ (or ").  It seems safer to punt on
+    # "..." for now rather than attempting to dequote or something.
+    $author =~ s#,.*##ms unless $author =~ m/"/;
     my $date = cmdoutput qw(date), '+%s %z', qw(-d), getfield($clogp,'Date');
     my $authline = "$author $date";
     $authline =~ m/$git_authline_re/o or
@@ -2312,6 +2324,7 @@ sub generate_commits_from_dsc () {
 
     my $authline = clogp_authline $clogp;
     my $changes = getfield $clogp, 'Changes';
+    $changes =~ s/^\n//; # Changes: \n
     my $cversion = getfield $clogp, 'Version';
 
     if (@tartrees) {
@@ -4429,7 +4442,12 @@ sub cmd_clone {
 }
 
 sub branchsuite () {
-    my $branch = cmdoutput_errok @git, qw(symbolic-ref HEAD);
+    my @cmd = (@git, qw(symbolic-ref -q HEAD));
+    my $branch = cmdoutput_errok @cmd;
+    if (!defined $branch) {
+	$?==256 or failedcmd @cmd;
+	return undef;
+    }
     if ($branch =~ m#$lbranch_re#o) {
 	return $1;
     } else {
diff --git a/dgit-nmu-simple.7.pod b/dgit-nmu-simple.7.pod
index 3ebc68a4..0d2525fc 100644
--- a/dgit-nmu-simple.7.pod
+++ b/dgit-nmu-simple.7.pod
@@ -56,10 +56,10 @@ consult the appropriate C<dgit-maint-*(7)> workflow tutorial,
 =head1 WHAT KIND OF CHANGES AND COMMITS TO MAKE
 
 When preparing an NMU, the git commits you make on the dgit branch
-should be simple linear series of commmits with good commit messages.
+should be simple linear series of commits with good commit messages.
 The commit messages will be published in various ways,
 including perhaps being used as the cover messages for
-genrated quilt patches.
+generated quilt patches.
 
 Do not make merge commits.
 Do not try to rebase to drop patches - if you need to revert a
diff --git a/dgit-user.7.pod b/dgit-user.7.pod
index d7b61098..aacdf4d4 100644
--- a/dgit-user.7.pod
+++ b/dgit-user.7.pod
@@ -232,7 +232,7 @@ that are in debian/patches before you do anything else!
 
 Debian package builds are often quite messy:
 they may modify files which are also committed to git,
-or leave outputs and teporary files not covered by C<.gitignore>.
+or leave outputs and temporary files not covered by C<.gitignore>.
 
 If you always commit,
 you can use
diff --git a/tests/lib-import-chk b/tests/lib-import-chk
index ee33cbef..88984c1b 100644
--- a/tests/lib-import-chk
+++ b/tests/lib-import-chk
@@ -1,4 +1,15 @@
 
+t-import-chk-authorship () {
+	perl -ne 'print $1,"\n" if m/^ -- (\S.*\>)  /' debian/changelog \
+		| sort -u \
+		> $tmp/authorship.changelog
+	${import_chk_changelog_massage:-:} $tmp/authorship.changelog
+	git log --pretty=format:'%an <%ae>%n%cn <%ce>' \
+		| sort -u \
+		> $tmp/authorship.commits
+	diff $tmp/authorship.{changelog,commits}
+}
+
 t-import-chk1 () {
 	p=$1
 	v=$2
@@ -15,6 +26,8 @@ t-import-chk2() {
 
 	cd $p
 
+	t-import-chk-authorship
+
 	git branch first-import
 
 	m='Commit for import check'
diff --git a/tests/tests/import-maintmangle b/tests/tests/import-maintmangle
new file mode 100755
index 00000000..31a5f88e
--- /dev/null
+++ b/tests/tests/import-maintmangle
@@ -0,0 +1,41 @@
+#!/bin/bash
+set -e
+. tests/lib
+. $troot/lib-import-chk
+
+t-tstunt-parsechangelog
+
+t-select-package example
+v=1.0
+t-worktree $v
+
+cd $p
+
+dsc=${p}_${v}.dsc
+
+chk () {
+	local perl="$1"
+	local unperl="$2"
+	git checkout master~0
+	perl -i -pe "next unless m/^ -- /; $perl" debian/changelog
+	git commit --allow-empty -a -m "perl $perl"
+	(cd ..; dpkg-source -i\.git -I.git -b $p)
+	t-dgit import-dsc ../$dsc +x
+	git checkout x~0
+	t-import-chk-authorship
+}
+
+massage () {
+	perl -i~ -pe "$unperl" "$1"
+}
+
+import_chk_changelog_massage=massage
+
+chk
+
+chk 	's/Ian Jackson/Ian Jackson, SPQR/' \
+	's/Ian Jackson, SPQR/Ian Jackson SPQR/'
+
+chk 	's/Ian Jackson/"Ian Jackson, SPQR"/'
+
+t-ok
diff --git a/tests/tests/quilt-useremail b/tests/tests/quilt-useremail
new file mode 100755
index 00000000..f079395e
--- /dev/null
+++ b/tests/tests/quilt-useremail
@@ -0,0 +1,27 @@
+#!/bin/bash
+set -e
+. tests/lib
+
+t-tstunt-parsechangelog
+t-archive example 1.0-1
+t-worktree 1.0
+t-git-none
+
+cd $p
+
+git checkout quilt-tip-2
+
+t-dgit -wgf fetch
+
+oe=other.email@example.com
+on='Hannibal Barca'
+
+git config --local user.email "$oe"
+git config --local user.name  "$on"
+
+t-dgit -wgf --quilt=smash quilt-fixup
+
+git show | fgrep "$oe"
+git show | fgrep "$on"
+
+t-ok

Reply to: