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

Re: Including build metadata in packages



On Sun, 13 Feb 2022 at 14:13:10 -0800, Vagrant Cascadian wrote:
> Obviously, this would interfere with any meaningful reproducible builds
> testing for any package that did something like this. Ideally metadata
> like this about a build should *not* be included in the .deb files
> themselves.

Relatedly, I would like to be able to capture some information about
builds even if (perhaps especially if) the build fails. It might make
sense to combine that with what you're looking at. It doesn't seem
ideal that for a successful build, the maintainer can recover detailed
results via a .deb (at a significant reproducibility cost), but for a
failing build - perhaps one that fails as a result of test regressions
- they get no information other than what's in the log. If anything,
these artifacts seem *more* important for failing builds.

Some prior art for this:

In any autopkgtest test-case, in addition to the machine-readable result
(exit status, and depending on test flags, maybe also whether stderr is 0
bytes or >= 1 byte), and the generic human-readable log (stdout/stderr),
tests can drop arbitrary files into $AUTOPKGTEST_ARTIFACTS and they will
be saved in a directory or tarball by the test infrastructure.
ci.debian.net keeps test artifacts for a while and then discards them
(they are not kept indefinitely).

Lots of upstream build systems output detailed test results in some
way: for example Autotools test-suite.log, *.log and *.trs, Meson
meson-logs/**, and various packages like librsvg and gtk4 that drop logs,
images etc. into somewhere under the build directory and/or /tmp for later
analysis if a test fails. At the moment, anything written to these places
and not recorded in the build's stdout/stderr just gets thrown away.

In at least librsvg and gtk4, there is Debian-specific code to grab the
results of failing "reftests" (drawing the same thing in two different
ways that should end up equivalent, and comparing the resulting PNG
images for equality), uuencode them and output them into the log for later
inspection: this is particularly important when a maintainer is assessing
whether a reftest result is "close enough" (e.g. font hinting is off
by a few pixels due to different rounding) or unacceptable (e.g. text
is unreadable or in the wrong place). Getting this out via uuencode is
practically annoying, but it's better than nothing...

In Gitlab-CI, there's a simple, declarative way to ask Gitlab to save
certain files from the CI job's container and store them in a zip file
for later inspection. For example, for a Meson build this could look like:

    artifacts:
      when: always
      paths:
        - _build/meson-logs
        - _build/tests/reftests/*.expected.png
        - _build/tests/reftests/*.actual.png

> * output plaintext data to the build log
> 
> Some of these log files are large (>13MB? per architecture, per package
> build) and would greatly benefit from compression...
> 
> How large is too large for this approach to work?
> 
> Relatively simple to implement (at least for plain text logs), but
> potentially stores a lot of data on the buildd infrastructure...

This has the advantage that it can work equally well for failing and
successful builds, and doesn't need any special support in either the
buildd infrastructure or dak.

For packages like gtk4 and librsvg that are quite visual, it would be
very useful to be able to record images (that is, potentially quite
large binary files) and not just text: uuencoding them is a workaround,
but screen-scraping the logs to get the uuencoded binary PNGs out is
not a great start to a debugging session. So far, I've been lucky and
all the failing reftests have had relatively small output...

> * Selectively filter out known unreproducible files
> 
> This adds complexity to the process of verification; you can't beat the
> simplicty of comparing checksums on two .deb files.
> 
> With increased complexity comes increased opportunity for errors, as
> well as maintenance overhead.
> 
> RPM packages, for example, embed signatures in the packages, and these
> need to be excluded for comparison.
> 
> I vaguely recall at least one case where attempting something like this
> in the past and resulting in packages incorrectly being reported as
> reproducible when the filter was overly broad...
> 
> Some nasty corner cases probably lurk down this approach...

A significant disadvantage of this approach is that it will only work
for successful builds: you can't use it to record more information about
a FTBFS caused by build-time test failures, unless you are willing to
let packages with build-time test failures into the archive (at which
point people will start using them or build-depending on them, which
we don't really want for packages that have failed the QA checks that
were meant to stop them from being shipped if they're broken/unusable,
either on a particular architecture or in general).

I personally don't like this: as you say, it's difficult to beat the
simplicity of "if the content isn't identical then the package is
unreproducible".

> * Split build metadata into a separate .deb file
> 
> Some of the similar problems of the previous, though maybe a little
> easier to get a reliable exclusion pattern? Wouldn't require huge
> toolchain changes.
> 
> I would expect that such packages be not actually dependend on by any
> other packages, and *only* contain build metadata. Maybe named
> SOURCEPACKAGE-buildmetadata-unreproducible.deb ... or.... ?
> 
> Not beautiful or elegant, but maybe actually achievable for bookworm
> release cycle?

This is not exactly elegant, but it wouldn't need infrastructural changes
(we'd just have to define a naming convention for packages that are
intentionally unreproducible).

Another disadvantage of this approach is that it will only work for
successful builds, as above.

This will require each package that adopts it to go through NEW, unless
the ftp team are willing to special-case packages that match the pattern
to be accepted automatically (like they do for automatic -dbgsym packages).

> * Split build metadata into a separate file or archive
> 
> Some of the debian-installer packages generate tarballs that are not
> .deb files and are included in the .changes files when uploading to the
> archive; making a similar generalized option for other packages to put
> build metadata into a separate artifact might be workable approach,
> although this would presumably require toolchain changes in dpkg and dak
> at the very least, and might take a couple release cycles, which
> is... well, debian.
> 
> The possibility of bundling up .buildinfo files into this metadata too,
> while taking some changes in relevent dpkg, dak, etc. tooling, might in
> the long term be worth exploring.
> 
> There was a relevent bug report in launchpad:
> 
>   https://bugs.launchpad.net/launchpad/+bug/1845159
> 
> This seems like the best long-term approach, but pretty much *only* a
> long-term approach...

I think even if we do one of the other approaches as a stopgap, we'll
want this in the long term.

There are two approaches that could be taken to this. One is to use
BYHAND, as Paul Wise already discussed. This would require action from the
ftp team and dak (I think), but nothing special in sbuild or the buildd
infrastructure.

However, I'd prefer it if this was output from the build alongside the log,
instead of being exported via the .changes file, so that failing builds
can also produce artifacts, to help the maintainer and/or porters to
figure out why the build failed. This would require action in sbuild and
the buildd infrastructure, but not in dak, because handling build logs is
not dak's job (and I don't think handling things like the binutils test
results should be dak's job either).

Here's a straw-man spec, which I have already prototyped in
<https://salsa.debian.org/debian/sbuild/-/merge_requests/14>:

    Each whitespace-separated token in the Build-Artifacts field represents
    a filename pattern in the same simplified shell glob syntax used in
    "Machine-readable debian/copyright file", version 1.0.

    If the pattern matches a directory, its contents are included in
    the artifacts, recursively. If a pattern matches another file type,
    it is included in the artifacts as-is. If a pattern does not match
    anything, nothing is included in the artifacts: this may be diagnosed
    with a warning, but is not an error.

    If a pattern matches files outside the build directory, is an absolute
    path or contains ".." segments,  build tools may exclude those files
    from the artifacts.

    Build tools should collect the artifacts that match the specified
    patterns, for example in a compressed tar archive, and make them
    available alongside the build log for inspection. The artifacts should
    usually be collected regardless of whether the build succeeds or fails.

    The Build-Artifacts field is not copied into the source package control
    file (dsc(5)), binary package control file (deb-control(5)),
    changes file (deb-changes(5)) or any other build results.

(To prototype this without dpkg supporting it, X-Build-Artifacts would be
appropriate, with none of the XS-, XB-, XC- prefixes.)

For example, a package using Meson with recent debhelper versions would
typically use:

    Build-Artifacts: obj-*/meson-logs

or a package using recursive Automake might use:

    Build-Artifacts:
     config.log
     tests/*.log
     tests/*.trs
     tests/reftests/*.png

Does that sound like what you had in mind?

In practice, if a build currently produces a log file with a name like
foo_1.2-3_amd64.build, I think it would make sense for it to produce an
accompanying tarball foo_1.2-3_amd64-artifacts.tar.xz or similar. This
could be pushed down into dpkg, but I think it might actually make more
sense in sbuild and pbuilder.

To be useful on buildds, the buildd infrastructure would have to pick up
these artifacts from sbuild and make them available for download alongside
the build logs. I don't know the buildd infrastructure, so I'm not
volunteering for that part.

    smcv


Reply to: