Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Sean Whitton
Package: dgit-infrastructure
Version: 8.4

Hello,

swayidle was not actually ever rejected from NEW but was instead dcut
from DEFERRED and uploaded again a few times.

Shouldn't --deliberately-include-questionable-history be required only
once?  Shouldn't dgit-infrastructure learn that the commits it thinks
are tainted aren't actually tainted?

Total 47 (delta 8), reused 19 (delta 0)
remote: gpgv: Signature made Mon 04 Mar 2019 05:36:37 PM UTC
remote: gpgv:                using RSA key 9B917007AE030E36E4FC248B695B7AE4BF066240
remote: gpgv: Good signature from "Sean Whitton <[hidden email]>"
remote: gpgv:                 aka "Sean Whitton <[hidden email]>"
remote: gpgv:                 aka "Sean Whitton <[hidden email]>"
remote: gpgv:                 aka "Sean Whitton <[hidden email]>"
remote: gpgv:                 aka "Sean Whitton <[hidden email]>"
remote: To /srv/dgit.debian.org/dispatch-dir/distro=debian/repos/_tmp/swayidle_prospective
remote:  * [new branch]      a719af136406276be15bd51270c2c36cf42575cc -> refs/dgit/sid
remote:  * [new tag]         e4a96dfac0f651a6f46a2a255dfda3e806c6115a -> archive/debian/1.2-2
remote:  * [new tag]         a1d2c059f0157a8e307b9202378d7466dd0015ab -> debian/1.2-2
remote: To /srv/dgit.debian.org/dispatch-dir/distro=debian/repos/_tmp/swayidle_prospective
remote:  * [new branch]      a719af136406276be15bd51270c2c36cf42575cc -> master
remote:
remote: History contains tainted commit 2ddf9061f6aba7ee03c7da1d5dec6a4225c2e77e
remote: Taint recorded at time 2019-02-09 21:39:02 Z for package swayidle
remote: Reason: tag archive/debian/1.2-1 referred to this object in git tree but all previously pushed versions were found to have been removed from NEW (ie, rejected) (or never arrived)
remote: Could be forced using --deliberately.  Consult documentation.
remote:
remote: Rejecting push due to questionable history.
dgit-repos-server: policy hook failed (or rejected) (256)
remote: dgit-repos-server: policy hook failed (or rejected) (256)
To git+ssh://push.dgit.debian.org/dgit/debian/repos/swayidle.git
 ! [remote rejected] a719af136406276be15bd51270c2c36cf42575cc -> refs/dgit/sid (pre-receive hook declined)
 ! [remote rejected] archive/debian/1.2-2 -> archive/debian/1.2-2 (pre-receive hook declined)
 ! [remote rejected] debian/1.2-2 -> debian/1.2-2 (pre-receive hook declined)
error: failed to push some refs to 'git+ssh://[hidden email]/dgit/debian/repos/swayidle.git'
dgit: failed command: git -c push.followTags=false push 'git+ssh://[hidden email]/dgit/debian/repos/swayidle.git' a719af136406276be15bd51270c2c36cf42575cc:refs/dgit/sid refs/tags/archive/debian/1.2-2 refs/tags/debian/1.2-2

--
Sean Whitton

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Ian Jackson-2
Sean Whitton writes ("Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push"):
> Package: dgit-infrastructure
> Version: 8.4
...
> swayidle was not actually ever rejected from NEW but was instead dcut
> from DEFERRED and uploaded again a few times.

AFAIAA there is no way for dgit-infrastructure or dgit to know whether
something was dcut or rejected.

> Shouldn't --deliberately-include-questionable-history be required only
> once?  Shouldn't dgit-infrastructure learn that the commits it thinks
> are tainted aren't actually tainted?

Each time the thing is cut and repushed, presumably there are new
commits that might be tainted.

Ie,
   push ...A
   cut              now ...A is tainted
   push ...A+B      needs reassurance that A is OK
   cut              now ...B is tainted
   push ...A+B      needs reassurance that A+B is OK
etc.

Don't you think ?

Ian.

Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Sean Whitton
Hello,

On Mon 04 Mar 2019 at 06:03PM +00, Ian Jackson wrote:

> AFAIAA there is no way for dgit-infrastructure or dgit to know whether
> something was dcut or rejected.

Right.

>> Shouldn't --deliberately-include-questionable-history be required only
>> once?  Shouldn't dgit-infrastructure learn that the commits it thinks
>> are tainted aren't actually tainted?
>
> Each time the thing is cut and repushed, presumably there are new
> commits that might be tainted.
>
> Ie,
>    push ...A
>    cut              now ...A is tainted
>    push ...A+B      needs reassurance that A is OK
>    cut              now ...B is tainted
>    push ...A+B      needs reassurance that A+B is OK
> etc.
>
> Don't you think ?
Once I --deliberately the 1.2-1 upload, and dgit sees that it was
ACCEPTed, can't it safely untaint everything in the 1.2-1 history?

That way, eventually, all commits will be marked as untainted and I
won't have to keep passing the option until the end of time.

If not, I think there needs to be some moderately standard way to
request that the whole current history of a package on dgit-repos be
marked as untainted.

--
Sean Whitton

signature.asc (847 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Ian Jackson-2
In reply to this post by Sean Whitton
Ian Jackson writes ("Re: Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push"):
> Not really in a position to look at this properly but I think I
> understand now and I think you have indeed identified a bug.

I have spent a while trying to reproduce this.  I found that there is
indeed code which is supposed to remove taints when they are
overridden.

It wasn't tested at all.  But I wrote a sketch of a test, which I
thought would resemble what happened to this package, and the
mechanism seems to work.  See attached.

Can you tell me the exact sequence of events up to the first
unwarranted push rejection, with times where you have them ?

Ian.


#!/bin/bash
set -e
. tests/lib

t-tstunt-parsechangelog

t-debpolicy
t-prep-newpackage example 1.0

cd $p
revision=1
git tag start
t-dgit setup-mergechangelogs


: upload

t-dgit push-source --new

: cut

rm $tmp/incoming/*
t-archive-none example

touch -d 'now -1 day' $tmp/git/example*

t-policy-periodic # xxx maybe want test both with and without this

: edit
t-commit 'edit after cut'

: push, needs --deliberately

t-expect-push-fail \
 'all previously pushed versions were found to have been removed from NEW' \
t-dgit push-source --new

t-dgit push-source --new --deliberately-include-questionable-history

t-archive-process-incoming new

: t-policy-periodic # xxx maybe want test both with and without this

: accept

mv -f $tmp/aq/package.{new,sid}.$p
t-aq-archive-updated sid $p

: t-policy-periodic # xxx maybe want test both with and without this

t-git-dir-time-passes

: 3rd push, no deliberately

t-commit 'edit after accept'
t-dgit push-source

t-ok



--
Ian Jackson <[hidden email]>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.
Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Ian Jackson-2
Control: tag -1 + moreinfo

> I have spent a while trying to reproduce this.  I found that there is
> indeed code which is supposed to remove taints when they are
> overridden.
>
> It wasn't tested at all.  But I wrote a sketch of a test, which I
> thought would resemble what happened to this package, and the
> mechanism seems to work.  See attached.
>
> Can you tell me the exact sequence of events up to the first
> unwarranted push rejection, with times where you have them ?

Ian.

Reply | Threaded
Open this post in threaded view
|

Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push

Ian Jackson-2
In reply to this post by Ian Jackson-2
Control: clone -1 -2
Control: reopen -2
Control: retitle -2 fix test for taint removal
Control: severity -2 normal

Ian Jackson writes ("Re: Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push"):

> Ian Jackson writes ("Re: Bug#923732: dgit-infrastructure: --deliberately-include-questionable-history required for every single push"):
> > Not really in a position to look at this properly but I think I
> > understand now and I think you have indeed identified a bug.
>
> I have spent a while trying to reproduce this.  I found that there is
> indeed code which is supposed to remove taints when they are
> overridden.
>
> It wasn't tested at all.  But I wrote a sketch of a test, which I
> thought would resemble what happened to this package, and the
> mechanism seems to work.  See attached.

This is still floating about, unfinished, in a branch on my laptop.

Ian.