Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian....@packages.debian.org
Usertags: pu

dgit 3.10 has a number of bugs which need to be fixed urgently.  I
considered them RC for buster and uploaded 3.11 to fix them there.
They need to be fixed in stable too.

3.11 has only stable-targeted bugfixes.  I would like to rebuild it
for stretch-p-u as 3.11~deb9, ASAP.


For full details of the changes, please see:

  git clone https://git.dgit.debian.org/dgit
  cd dgit
  git log archive/debian/3.10..archive/debian/3.11

NB that that doesn't contain the proposed changelog entry for
3.11~deb9.  That *is* in the attached combined diff.


The changes I'm here proposing for stable are to fix these bugs (along
with, in many cases, corresponding tests):

#857694 "Died at /usr/bin/dgit line 2196." with .xz files

   This critical bug makes dgit clone and dgit fetch fail for many
   current packages.  It is due to dgit not tolerating the compressor
   dying with SIGPIPE.  That is an expected situation which ought not
   to cause dgit to bomb out.

#867189 combined suite "foo,-security" does not work
#867189 cloning a combined suite deletes the working directory

   Combined suites are an important feature (recommended in
   dgit-user(7) and required by downstream users).  These bugs make
   this feature quite hard to use (although there are workarounds).
   Both fixes are to the combined suite code (so cannot break other
   use cases).  Each fix is a single line, but in the case of #867189
   quite subtle (as discussed in the commit messages).

#865863 Cope with newer git which hates --local outside a working tree

   Newer git than is in stretch breaks "dgit clone" completely.  I
   consider this a serious bug even for stretch because many people -
   especially keen git users - will install a newer git (from Debian
   backports, or from upstream).  The fix is annoyingly textuallly
   large, but conceptually simple.

#867702 Cope with new git-receive-pack which has quarantine feature

   Newer git than is in stretch breaks the backend
   dgit-infrastructure.  See
      https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=867702#10
   for discussion.  The workaround is necessary to get the test suite
   to pass on buster, and I would prefer to send to stretch an
   identical package of bugfixes to that I have sent to buster in dgit
   3.11.  The change is confined to dgit-infrastructure; for users
   using the dgit-infrastructure package from stretch, the arguments
   about them maybe wanting newer git apply, again.

#858054 clone fails if git does not create info/ (eg due to templatedir)

   This bug is annoying in this use case and risks breaking with
   future git versions.  The fix is extremely simple and low-risk.

#867603 dgit-badcommit-fixup: does not respect core.sharedRepository

   This causes this script to, in a very common case, leave the shared
   repo with broken permissions which prevent other pushes.  I have
   fixed this in a way that is confined to the prticular script, so
   users of dgit proper (and who aren't affected by the bug which this
   script exists to fix) are not at risk from this change.

   However, an analogous bug affects dgit itself, because dgit also
   manipulates the user's object store via a special-purpose tree,
   private to dgit.  These object store manipulations ought to honour
   core.sharedRepository (and some other options which control the
   object store).

(I have also found a couple of other less-critical issues which ought
to be fixed in stable, but which can be dealt with in a more leisurely
fashion.  I won't discuss those further in this bug submission.)


Thanks for your attention,
Ian.


-- System Information:
Debian Release: 9.0s
  APT prefers stable-debug
  APT policy: (500, 'stable-debug'), (500, 'stable')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 4.9.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 
(charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: sysvinit (via /sbin/init)
diff --git a/debian/changelog b/debian/changelog
index 392f1291..51e03acc 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,36 @@
+dgit (3.11~deb9) stable; urgency=high
+
+  * Rebuild and upload to stretch.
+
+ -- Ian Jackson <ijack...@chiark.greenend.org.uk>  Tue, 11 Jul 2017 09:28:15 
+0100
+
+dgit (3.11) unstable; urgency=high
+
+  Important bugfixes to dgit:
+  * Fix rpush+buildinfo: Transfer buildinfos for signing.  Closes:#867693.
+  * Cope if the archive server sends an HTTP redirect,
+    by passing -L to curl.  Closes:#867185,#867309.
+  * Cope with newer git which hates --local outside a tree.  Closes:#865863.
+  * rpush: Honour local git config from build host working tree.
+  * Tolerate compressor terminating with SIGPIPE.  Closes:#857694.
+  * Honour more pre-tree git config options in our private trees sharing
+    the user's object store.  In particular, core.sharedRepository.
+    Prompted by #867603.
+  * Clone multisuite works even without --no-rm-on-error.  Closes:#867434.
+  * Work if "git init" does not create $GIT/info.  Closes:#858054.
+  * Actually understand foo,-security (!)  Closes:#867189.
+
+  Important bugfixes to other components:
+  * dgit-badcommit-fixup: Honour core.sharedRepository.   Closes:#867603.
+  * infrastructure: Cope with new git-receive-pack which has
+    quarantine feature: ie, work around #867702.
+
+  Test suite:
+  * Cope with git restricting ext:: protocols.
+  * multisuite: Test clone without --rm-on-error.
+
+ -- Ian Jackson <ijack...@chiark.greenend.org.uk>  Sat, 08 Jul 2017 22:40:15 
+0100
+
 dgit (3.10) unstable; urgency=medium
 
   Bugfixes:
diff --git a/dgit b/dgit
index 6b1201e7..ccf50094 100755
--- a/dgit
+++ b/dgit
@@ -100,7 +100,7 @@ our $rewritemap = 'dgit-rewrite/map';
 
 our (@git) = qw(git);
 our (@dget) = qw(dget);
-our (@curl) = qw(curl);
+our (@curl) = (qw(curl --proto-redir), '-all,http,https', qw(-L));
 our (@dput) = qw(dput);
 our (@debsign) = qw(debsign);
 our (@gpg) = qw(gpg);
@@ -316,6 +316,9 @@ sub gbp_pq {
 #  > param tagformat old|new
 #  > param maint-view MAINT-VIEW-HEAD
 #
+#  > param buildinfo-filename P_V_X.buildinfo   # zero or more times
+#  > file buildinfo                             # for buildinfos to sign
+#
 #  > previously REFNAME=OBJNAME       # if --deliberately-not-fast-forward
 #                                     # goes into tag, for replay prevention
 #
@@ -332,6 +335,9 @@ sub gbp_pq {
 #  [etc]
 #  < data-block NBYTES    [transfer of signed changes]
 #  [etc]
+#  < data-block NBYTES    [transfer of each signed buildinfo
+#  [etc]                   same number and order as "file buildinfo"]
+#  ...
 #  < files-end
 #
 #  > complete
@@ -560,6 +566,9 @@ sub nextarg {
     return scalar shift @ARGV;
 }
 
+sub pre_help () {
+    no_local_git_cfg();
+}
 sub cmd_help () {
     print $helpmsg or die $!;
     exit 0;
@@ -698,6 +707,11 @@ sub cfg {
        "$us: distro or suite appears not to be (properly) supported";
 }
 
+sub no_local_git_cfg () {
+    # needs to be called from pre_*
+    @gitcfgsources = grep { $_ ne 'local' } @gitcfgsources;
+}
+
 sub access_basedistro__noalias () {
     if (defined $idistro) {
        return $idistro;
@@ -1699,7 +1713,10 @@ 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)) {
+    foreach my $copy (qw(user.email user.name user.useConfigOnly
+                         core.sharedRepository
+                         core.compression core.looseCompression
+                         core.bigFileThreshold core.fsyncObjectFiles)) {
        my $v = $gitcfgs{local}{$copy};
        next unless $v;
        runcmd qw(git config), $copy, $_ foreach @$v;
@@ -2167,7 +2184,7 @@ sub generate_commits_from_dsc () {
                if defined $compr_ext && !defined $cname;
            my $compr_proc =
                new Dpkg::Compression::Process compression => $cname;
-           my @compr_cmd = $compr_proc->get_uncompress_cmdline();
+           @compr_cmd = $compr_proc->get_uncompress_cmdline();
            my $compr_fh = new IO::Handle;
            my $compr_pid = open $compr_fh, "-|" // die $!;
            if (!$compr_pid) {
@@ -2194,7 +2211,7 @@ sub generate_commits_from_dsc () {
        !$? or failedcmd @tarcmd;
 
        close $input or
-           (@compr_cmd ? failedcmd @compr_cmd
+           (@compr_cmd ? ($?==SIGPIPE || failedcmd @compr_cmd)
             : die $!);
        # finally, we have the results in "tarball", but maybe
        # with the wrong permissions
@@ -2513,7 +2530,7 @@ sub complete_file_from_dsc ($$;$) {
        $fi->{Digester}->reset();
        $fi->{Digester}->addfile(*F);
        F->error and die $!;
-       my $got = $fi->{Digester}->hexdigest();
+       $got = $fi->{Digester}->hexdigest();
        return $got eq $fi->{Hash};
     };
 
@@ -3363,6 +3380,7 @@ END
        return;
     }
     my $af = ".git/info/attributes";
+    ensuredir '.git/info';
     open GAO, "> $af.new" or die $!;
     print GAO <<END or die $!;
 *      dgit-defuse-attrs
@@ -3481,6 +3499,7 @@ sub fork_for_multisuite ($) {
     $before_fetch_merge->();
 
     foreach my $tsuite (@suites[1..$#suites]) {
+       $tsuite =~ s/^-/$cbasesuite-/;
        my $csubsuite = multisuite_suite_child($tsuite, \@mergeinputs,
                                               sub {
             @end = ();
@@ -3595,6 +3614,9 @@ END
 }
 
 sub clone ($) {
+    # in multisuite, returns twice!
+    # once in parent after first suite fetched,
+    # and then again in child after everything is finished
     my ($dstdir) = @_;
     badusage "dry run makes no sense with clone" unless act_local();
 
@@ -3606,7 +3628,7 @@ sub clone ($) {
         printdebug "multi clone after fetch merge\n";
        clone_set_head();
        clone_finish($dstdir);
-       exit 0;
+       return;
     }
     printdebug "clone main body\n";
 
@@ -4296,6 +4318,14 @@ END
        responder_send_command("param maint-view $maintviewhead");
     }
 
+    # Perhaps send buildinfo(s) for signing
+    my $changes_files = getfield $changes, 'Files';
+    my @buildinfos = ($changes_files =~ m/ .* (\S+\.buildinfo)$/mg);
+    foreach my $bi (@buildinfos) {
+       responder_send_command("param buildinfo-filename $bi");
+       responder_send_file('buildinfo', "$buildproductsdir/$bi");
+    }
+
     if (deliberately_not_fast_forward) {
        git_for_each_ref(lrfetchrefs, sub {
            my ($objid,$objtype,$lrfetchrefname,$reftail) = @_;
@@ -4366,9 +4396,10 @@ If you need to change the package, you must use a new 
version number.
 END
     if ($we_are_responder) {
        my $dryrunsuffix = act_local() ? "" : ".tmp";
+       my @rfiles = ($dscpath, $changesfile);
+       push @rfiles, map { "$buildproductsdir/$_" } @buildinfos;
        responder_receive_files('signed-dsc-changes',
-                               "$dscpath$dryrunsuffix",
-                               "$changesfile$dryrunsuffix");
+                               map { "$_$dryrunsuffix" } @rfiles);
     } else {
        if (act_local()) {
            rename "$dscpath.tmp",$dscpath or die "$dscfn $!";
@@ -4394,6 +4425,9 @@ END
     responder_send_command("complete");
 }
 
+sub pre_clone () {
+    no_local_git_cfg();
+}
 sub cmd_clone {
     parseopts();
     my $dstdir;
@@ -4527,7 +4561,7 @@ sub cmd_push {
 
 #---------- remote commands' implementation ----------
 
-sub cmd_remote_push_build_host {
+sub pre_remote_push_build_host {
     my ($nrargs) = shift @ARGV;
     my (@rargs) = @ARGV[0..$nrargs-1];
     @ARGV = @ARGV[$nrargs..$#ARGV];
@@ -4557,11 +4591,14 @@ sub cmd_remote_push_build_host {
         " but invocation host has $vsnwant"
        unless defined $protovsn;
 
-    responder_send_command("dgit-remote-push-ready $protovsn");
     changedir $dir;
+}
+sub cmd_remote_push_build_host {
+    responder_send_command("dgit-remote-push-ready $protovsn");
     &cmd_push;
 }
 
+sub pre_remote_push_responder { pre_remote_push_build_host(); }
 sub cmd_remote_push_responder { cmd_remote_push_build_host(); }
 # ... for compatibility with proto vsn.1 dgit (just so that user gets
 #     a good error message)
@@ -4601,6 +4638,9 @@ sub i_method {
     { no strict qw(refs); &{"${base}_${selector}"}(@args); }
 }
 
+sub pre_rpush () {
+    no_local_git_cfg();
+}
 sub cmd_rpush {
     my $host = nextarg;
     my $dir;
@@ -4719,7 +4759,7 @@ sub i_resp_want ($) {
     print RI "files-end\n" or die $!;
 }
 
-our ($i_clogp, $i_version, $i_dscfn, $i_changesfn);
+our ($i_clogp, $i_version, $i_dscfn, $i_changesfn, @i_buildinfos);
 
 sub i_localname_parsed_changelog {
     return "remote-changelog.822";
@@ -4736,6 +4776,31 @@ sub i_localname_dsc {
 }
 sub i_file_dsc { }
 
+sub i_localname_buildinfo ($) {
+    my $bi = $i_param{'buildinfo-filename'};
+    defined $bi or badproto \*RO, "buildinfo before filename";
+    defined $i_changesfn or badproto \*RO, "buildinfo before changes";
+    $bi =~ m{^\Q$package\E_[!-.0-~]*\.buildinfo$}s
+       or badproto \*RO, "improper buildinfo filename";
+    return $&;
+}
+sub i_file_buildinfo {
+    my $bi = $i_param{'buildinfo-filename'};
+    my $bd = parsecontrol "$i_tmp/$bi", $bi;
+    my $ch = parsecontrol "$i_tmp/$i_changesfn", 'changes';
+    if (!forceing [qw(buildinfo-changes-mismatch)]) {
+       files_compare_inputs($bd, $ch);
+       (getfield $bd, $_) eq (getfield $ch, $_) or
+           fail "buildinfo mismatch $_"
+           foreach qw(Source Version);
+       !defined $bd->{$_} or
+           fail "buildinfo contains $_"
+           foreach qw(Changes Changed-by Distribution);
+    }
+    push @i_buildinfos, $bi;
+    delete $i_param{'buildinfo-filename'};
+}
+
 sub i_localname_changes {
     defined $i_dscfn or badproto \*RO, "dsc (before parsed-changelog)";
     $i_changesfn = $i_dscfn;
@@ -4777,7 +4842,7 @@ sub i_want_signed_tag {
 sub i_want_signed_dsc_changes {
     rename "$i_dscfn.tmp","$i_dscfn" or die "$i_dscfn $!";
     sign_changes $i_changesfn;
-    return ($i_dscfn, $i_changesfn);
+    return ($i_dscfn, $i_changesfn, @i_buildinfos);
 }
 
 #---------- building etc. ----------
@@ -6332,6 +6397,9 @@ END
        "results are in in git ref $dstbranch";
 }
 
+sub pre_archive_api_query () {
+    no_local_git_cfg();
+}
 sub cmd_archive_api_query {
     badusage "need only 1 subpath argument" unless @ARGV==1;
     my ($subpath) = @ARGV;
@@ -6348,6 +6416,9 @@ sub repos_server_url () {
     my $url = access_giturl();
 }    
 
+sub pre_clone_dgit_repos_server () {
+    no_local_git_cfg();
+}
 sub cmd_clone_dgit_repos_server {
     badusage "need destination argument" unless @ARGV==1;
     my ($destdir) = @ARGV;
@@ -6357,6 +6428,9 @@ sub cmd_clone_dgit_repos_server {
     exec @cmd or fail "exec git clone: $!\n";
 }
 
+sub pre_print_dgit_repos_server_source_url () {
+    no_local_git_cfg();
+}
 sub cmd_print_dgit_repos_server_source_url {
     badusage "no arguments allowed to dgit print-dgit-repos-server-source-url"
        if @ARGV;
@@ -6738,7 +6812,6 @@ if ($ENV{$fakeeditorenv}) {
 
 parseopts();
 check_env_sanity();
-git_slurp_config();
 
 print STDERR "DRY RUN ONLY\n" if $dryrun_level > 1;
 print STDERR "DAMP RUN - WILL MAKE LOCAL (UNSIGNED) CHANGES\n"
@@ -6753,6 +6826,8 @@ $cmd =~ y/-/_/;
 my $pre_fn = ${*::}{"pre_$cmd"};
 $pre_fn->() if $pre_fn;
 
+git_slurp_config();
+
 my $fn = ${*::}{"cmd_$cmd"};
 $fn or badusage "unknown operation $cmd";
 $fn->();
diff --git a/dgit-badcommit-fixup b/dgit-badcommit-fixup
index 8b202c0a..3995ceb6 100755
--- a/dgit-badcommit-fixup
+++ b/dgit-badcommit-fixup
@@ -59,6 +59,17 @@ my $bare = `git rev-parse --is-bare-repository`;
 die "$? $!" if $?;
 chomp $bare or die;
 
+our @configs;
+foreach my $k (qw(core.sharedRepository)) {
+    $?=0; $!=0; my $v = `set -x; git config --local $k`;
+    if (defined $v && $?==0 && chomp $v) {
+       push @configs, [ $k, $v ];
+    } elsif (defined $v && $?==256 && $v eq '') {
+    } else {
+       die "git-config --local $k => $v $? $! ?";
+    }
+}
+
 sub getobj ($$) {
     my ($obj, $type) = @_;
     print GCFI $obj, "\n" or die $!;
@@ -163,6 +174,9 @@ sub edit_rewrite_map ($) {
     runcmd qw(git config gc.auto 0);
     runcmd qw(rm -rf .git/objects);
     symlink "../../objects", ".git/objects" or die $!;
+    foreach my $c (@configs) {
+       runcmd qw(git config), $c->[0], $c->[1];
+    }
 
     my %map;
 
diff --git a/infra/dgit-repos-server b/infra/dgit-repos-server
index 6131774e..55dc81ff 100755
--- a/infra/dgit-repos-server
+++ b/infra/dgit-repos-server
@@ -407,16 +407,21 @@ sub makeworkingclone () {
     rmtree "${workrepo}_fresh";
 }
 
+sub mkscript ($$) {
+    my ($path,$contents) = @_;
+    my $fh = new IO::File $path, O_WRONLY|O_CREAT|O_TRUNC, 0777
+       or die "$path: $!";
+    print $fh $contents or die "$path: $!";
+    close $fh or die "$path: $!";
+}
+
 sub setupstunthook () {
     my $prerecv = "$workrepo/hooks/pre-receive";
-    my $fh = new IO::File $prerecv, O_WRONLY|O_CREAT|O_TRUNC, 0777
-       or die "$prerecv: $!";
-    print $fh <<END or die "$prerecv: $!";
+    mkscript $prerecv, <<END;
 #!/bin/sh
 set -e
 exec $0 --pre-receive-hook $package
 END
-    close $fh or die "$prerecv: $!";
     $ENV{'DGIT_DRS_WORK'}= $workrepo;
     $ENV{'DGIT_DRS_DEST'}= $destrepo;
     printdebug " stunt hook set up $prerecv\n";
@@ -938,6 +943,17 @@ sub onwardpush () {
     my @cmdbase = (qw(git send-pack), $destrepo);
     push @cmdbase, qw(--force) if $policy & NOFFCHECK;
 
+    if ($ENV{GIT_QUARANTINE_PATH}) {
+       my $recv_wrapper = "$ENV{GIT_QUARANTINE_PATH}/dgit-recv-wrapper";
+       mkscript $recv_wrapper, <<'END';
+#!/bin/sh
+set -e
+unset GIT_QUARANTINE_PATH
+exec git receive-pack "$@"
+END
+       push @cmdbase, "--receive-pack=$recv_wrapper";
+    }
+
     my @cmd = @cmdbase;
     push @cmd, "$commit:refs/dgit/$suite",
               "$tagval:refs/tags/$tagname";
diff --git a/tests/lib-core b/tests/lib-core
index 6cdffeb1..d65a1ffe 100644
--- a/tests/lib-core
+++ b/tests/lib-core
@@ -24,6 +24,7 @@ t-set-using-tmp () {
        export GNUPGHOME=$tmp/nonexistent
        git config --global user.email 'dgit-t...@debian.example.net'
        git config --global user.name 'dgit test git user'
+       git config --global protocol.ext.allow always
 }
 
 t-filter-out-git-hyphen-dir () {
diff --git a/tests/tests/badcommit-rewrite b/tests/tests/badcommit-rewrite
index b7fc7011..3e2f37e7 100755
--- a/tests/tests/badcommit-rewrite
+++ b/tests/tests/badcommit-rewrite
@@ -5,7 +5,12 @@ set -e
 t-setup-import examplegit
 t-tstunt-parsechangelog
 
-cd example
+cd $tmp/git/$p.git
+git config core.sharedRepository true
+chmod -R g+w objects
+umask 022
+
+cd $tmp/example
 
 suite=stable
 
@@ -26,6 +31,9 @@ t-has-parent-or-is $rstable $badcommit
 fixup=${DGIT_BADCOMMIT_FIXUP-dgit-badcommit-fixup}
 
 cd $tmp/git/$p.git
+git gc --aggressive --prune=all
+rmdir objects/* ||:
+
 $fixup --real
 
 cd $tmp/$p
@@ -44,4 +52,11 @@ t-dgit fetch stable
 t-expect-fail "child $rstable lacks parent $badcommit" \
 t-has-parent-or-is $rstable $badcommit
 
+check_shared () {
+       find "$1" -perm -200 \! -perm -020 -ls |tee $tmp/badperm
+       test -f $tmp/badperm -a ! -s $tmp/badperm
+}
+
+check_shared $tmp/git/$p.git/objects
+
 t-ok
diff --git a/tests/tests/multisuite b/tests/tests/multisuite
index d39475be..fe655d02 100755
--- a/tests/tests/multisuite
+++ b/tests/tests/multisuite
@@ -32,7 +32,7 @@ multi-good
 
 cd ..
 
-t-dgit clone --no-rm-on-error $p stable,unstable ./$p.clone
+t-dgit clone $p stable,unstable ./$p.clone
 
 cd $p.clone
 

Reply via email to