Bug#867190: stretch-pu: package apt/1.4.7
On Sun, Jul 09, 2017 at 03:22:12PM +0100, Adam D. Barratt wrote:
> On Wed, 2017-07-05 at 09:38 +0200, Julian Andres Klode wrote:
> > On Wed, Jul 05, 2017 at 07:58:11AM +0200, Cyril Brulebois wrote:
> > > Control: tag -1 moreinfo
> > >
> > > Hi,
> > >
> > > Julian Andres Klode <jak@debian.org> (2017-07-04):
> > > > This release fixes several smaller bugs in the network connection.
> > > > Apart from the changes in the changelog, there also are some changes
> > > > to our travis CI integration and the test suite to make CI more
> > > > reliable (by fixing coverage errors messing up our tests), more future
> > > > proof (by using docker instead of mixing travis' ubuntu trusty with
> > > > wily and xenial packages...), and the CI now runs on stretch, so
> > > > that's good too :)
> > > >
> > > > All fixes are straight cherry-picks from unstable, just the travis
> > > > stuff had some changes (one less variant to build and stretch isntead
> > > > of testing), but that does not affect us.
> > >
> > > apt will need to migrate to testing before we could tentatively accept
> > > anything from stretch-new; but we're not there yet.
> >
> > grr, why did the bits email Adam sent then say:
> >
> > " * The bug you want to fix in stable must be fixed in unstable
> > already (and not waiting in NEW or the delayed queue)"
> >
> > I guess you'll see a lot more people trying to do stable updates
> > only in unstable if that's what people have been told is needed
> > - perhaps send a follow-up correction?
>
> There are two things here that I think may be getting confused.
>
> The requirement for accepting an update to p-u has always been that it
> doesn't affect unstable - either by the affected code not being present
> or (more commonly) by the bug having already been fixed in unstable.
> That requirement is to ensure that the fix gets at least some exposure
> before the point release, and because it's quicker and simpler to get
> any required follow-up fixes applied in unstable. We do of course want
> to fix to be available to users of testing asap, but that's not a
> blocker for the p-u request.
>
> In order for the fix to then get into stable itself, the version of the
> package in testing must be strictly higher than that of the p-u update.
> It's preferable if that occurs due to a more recent package having
> migrated to testing, but if necessary ftp-master will (at our request)
> force packages into testing and/or unstable during the point release, in
> order to avoid violating the archive's inter-suite version constraints.
Ah yeah, sorry, I forgot that testing was still at 1.4.6 (now 1.5~beta1
seems to have migrated).
>
> > > > [ David Kalnischkies ]
> > > > * use port from SRV record instead of initial port
> > > > => Might have picked the wrong port
> > > >
> > > > [ Julian Andres Klode ]
> > > > * Reset failure reason when connection was successful
> > > > => Some failures were only treated as warnings, not errors
> > >
> > > No bug reports with details for those?
> > [...]
> > >
> > > > * http: A response with Content-Length: 0 has no content
> > > >
> > > > => Downloading failed if server responded with Content-Length: 0, as
> > > > APT was waiting for content to read.
> > >
> > > No bug report with details?
> >
> > I discovered the last two while writing the TLS support in 1.5, so I did
> > not open up a bug report for each. I'm not sure how David noticed the
> > first one, but all of them seem to be important bugs.
>
> I'm okay with the fixes, but the changelog descriptions aren't
> particularly great. For instance, you've explained in this thread that
> the Content-Length: bug caused apt to wait (indefinitely?) for content
> that would never arrive, but that's not really clear (at least to me)
> from the changelog.
There are timeouts for everything, well one value, defaulting to
120s, that is, 2 minutes.
So what do you suggest? A simple rewording like this:
" * http: A response with Content-Length: 0 has no content."
=> " * http: Do not try to read content if Content-Length is 0"
" * Reset failure reason when connection was successful."
=>
" * Ignore errors from earlier connection attempts on successful connection"
(What about the " use port from SRV record instead of initial port."? It's
entirely clear if you know what a SRV record is, but otherwise...).
Or more details? For example, we could expand all changelog entries to
include (almost) the entire commit message (the changelog is generated
by gbp-dch), then we end up with this monster:
apt (1.4.7) unstable; urgency=medium
[ David Kalnischkies ]
* travis: ignore profiling warning in progress lines.
On Travis CI running tests with code coverage enabled sometimes
generates profiling lines, which we filter out for a while now,
but that misses lines generated showing progress still causing test
failures, so more sed logic is added in the hopes to ignore them.
* use port from SRV record instead of initial port.
An SRV record includes a portnumber to use with the host given, but apt
was ignoring the portnumber and instead used either the port given by
the user for the initial host or the default port for the service.
In practice the service usually runs on another host on the default
port, so it tends to work as intended and even if not and apt can't get
a connection there it will gracefully fallback to contacting the initial
host with the right port, so its a user invisible bug most of the time.
[ Robert Luberda ]
* fix a "critical" typo in old changelog entry.
This typo exposes a bug in apt-listchanges that prevents commands like
`apt-listchanges --show-all apt_*.deb' from showing the changelog.
(Closes: 866358)
[ Julian Andres Klode ]
* Reset failure reason when connection was successful.
When APT was trying multiple addresses, any later error
somewhere else would be reported with ConnectionRefused
or ConnectionTimedOut as the FailReason because that
was set by early connect attempts. This causes APT to
handle the failures differently, leading to some weirdly
breaking test cases (like the changed one).
* debian/gbp.conf: Set debian-branch to 1.4.y
* http: A response with Content-Length: 0 has no content.
APT considered any response with a Content-Length to have a
body, even if the value of the header was 0. A 0 length body
however, is equal to no body.
* travis: Migrate to Docker.
This is based on master, just with one less variant, and
stretch as the base image.
* Release 1.4.7 (LP: #1702326)
-- Julian Andres Klode <jak@debian.org> Sun, 09 Jul 2017 16:59:55 +0200
>
> > > > diff -Nru apt-1.4.6/Dockerfile apt-1.4.7/Dockerfile
> > > > --- apt-1.4.6/Dockerfile 1970-01-01 01:00:00.000000000 +0100
> > > > +++ apt-1.4.7/Dockerfile 2017-07-04 17:11:59.000000000 +0200
> > > > @@ -0,0 +1,11 @@
> > > > +FROM debian:stretch
> > > > +COPY . /tmp
> > > > +WORKDIR /tmp
> > > > +RUN sed -i s#://deb.debian.org#://cdn-fastly.deb.debian.org# /etc/apt/sources.list \
> > > > + && apt-get update \
> > > > + && adduser --home /home/travis travis --quiet --disabled-login --gecos "" --uid 1000 \
> > > > + && env DEBIAN_FRONTEND=noninteractive apt-get install build-essential ccache ninja-build expect curl git -q -y \
> > > > + && env DEBIAN_FRONTEND=noninteractive ./prepare-release travis-ci \
> > > > + && dpkg-reconfigure ccache \
> > > > + && rm -r /tmp/* \
> > > > + && apt-get clean
> > >
> > > Not documented in changelog. Not sure this addition belongs to stable
> > > anyway.
> >
> > Of course not, it does not affect debian at all, that's why we hide
> > it from the changelog - but did mention it in the email. We could
> > add them to the changelog, but this would only confuse users, as
> > this has zero effect on the package.
> >
> > We only need those changes (.travis.yml, Dockerfile, and testing framework)
> > to fix our continuous integration which was flaky all the time, mostly
> > failing, and might stop working at some point. It is not used in Debian
> > at all, it's strictly a "prepare the release" kind of fix.
>
> Whilst it may be true that the changes don't affect the Debian package,
> I'd expect them to be documented /somewhere/ as they are clearly changes
> relative to the previous upstream release (and not simply "release
> preparation" imho as other changes to version numbers etc are). There
> doesn't seem to be an upstream changelog, NEWS file or similar in apt,
> so I'm not sure where other than the Debian changelog you'd suggest that
> they should be documented.
>
The entries for that would be:
* travis: ignore profiling warning in progress lines
* travis: Migrate to Docker
The first one by David, the second by me, so this would end up
something like the following (order may be different, it's auto-generated
from git commits):
apt (1.4.7) stretch; urgency=medium
[ Robert Luberda ]
* fix a "critical" typo in old changelog entry (Closes: 866358)
[ David Kalnischkies ]
* use port from SRV record instead of initial port
* travis: ignore profiling warning in progress lines
[ Julian Andres Klode ]
* Reset failure reason when connection was successful
* debian/gbp.conf: Set debian-branch to 1.4.y
* http: Do not try reading content if Content-Length is 0
* travis: Migrate to Docker
* Release 1.4.7 (LP: #1702326)
-- Julian Andres Klode <jak@debian.org> Tue, 04 Jul 2017 17:11:59 +0200
I guess maybe the first "travis:" should be "test suite:" instead of "travis:"
and the second one could be "travis CI:" instead of "travis", so people know
what we're talking about?
This also reduces the "release" entry linking to launchpad a bit. I
think it's useful for us to have that tracking bug referenced no matter
how it ends up over there, as it provides more details (we could also
link this pu bug in there) about the upload, and the motivation behind
it. As I said, I can drop it, but it probably causes no harm and might
simplify things (potentially we can get to the point in Ubuntu where
I can just sync the package over, then it's immensely useful, as I avoid
uploads ;)).
--
Debian Developer - deb.li/jak | jak-linux.org - free software dev
| Ubuntu Core Developer |
When replying, only quote what is necessary, and write each reply
directly below the part(s) it pertains to ('inline'). Thank you.
Reply to: