Page MenuHomeFreeBSD

Refactor pkg bootstrap to fetch compression types
Needs ReviewPublic

Authored by zac_freebsdfoundation.org on Sep 21 2020, 6:18 PM.

Details

Reviewers
bdrewery
Summary

Add support for fetching different types of compression of the pkg
package. It first tries to fetch pkg.txz (the original file), and if
unsuccessful, will try to fetch pkg.tzst, pkg.tbz, pkg.tgz, and pkg.tar
before failing.

Since the package extractor is compression-agnostic, only the code that
fetches the package was necessary to refactor.

Test Plan

Build the package binary, force a pkg bootstrap and confirm that it has
the same behaviour as package from upstream. Remove or rename the first
extension in the extensions array (".txz"), confirm with GDB that the
other package extensions are attempted to be fetched, and result in
desired behaviour once each fetch fails.

Moving the .txz extension (the compression currently being used for the
package) to a different position in the extensions array, use GDB to
confirm the program attempts to fetch the extensions in the correct
order before successfully fetching the package with the .txz extension.

Use Valgrind to confirm that no memory additional memory was leaked
from this code change.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 33746
Build 30971: arc lint + arc unit

Event Timeline

emaste added inline comments.
usr.sbin/pkg/pkg.c
878

FreeBSD style is a 4 spaces for 2nd-level indents.

clang-format should be able to help here, have a look at the instructions for clang-formatting a git patch at https://clang.llvm.org/docs/ClangFormat.html and see how it works, and then put those instructions on https://wiki.freebsd.org/Phabricator

zac_freebsdfoundation.org marked an inline comment as done.
  • Update formatting with clang-format
  • Revert "Update formatting with clang-format"
  • Update formatting with git clang-format
usr.sbin/pkg/pkg.c
835

I'd be tempted to move this out to the global scope, make it static, drop the NULL terminator, then...

869–872

... loop here from 0 to nitems(extension) (macro provided by sys/param.h)

878–879

All paths through the oop after this either goto fetchfail or cleanup, so you could invert this condition and break out of the loop, dropping the rest of the loop's body out of it. Do we suspect that we could hit a failure mode where, e.g., pkg.txz exists but the rest of this won't line up and we need to try a different archive type?

usr.sbin/pkg/pkg.c
835

in discussion with @bapt we want to add also the .bsd extension, it's not used yet but he wants to use it as the generic extension for packages

  • Move array of package extensions to global scope
usr.sbin/pkg/pkg.c
878–879

I believe that the desired behavior for when pkg.txz is fetched but reaches a failure state for whatever reason to attempt to fetch the next file from the extensions array to attempt recovery.

@bapt and I had an out-of-band discussion about this, and I think we'd like to see a slightly different approach taken -- there's been somewhat of a trend to move away from pkg tooling having to know/guess compression type that a repo uses before you have sufficient information to *know*. See, e.g., the 'recent' (probably not that recent anymore) transition to just using meta.conf instead of compressed meta.txz.

That said, the 'ideal' solution we discussed doesn't seem to be much harder at all from a cursory glance: poudriere should just create a compression format agnostic symlink in /Latest to the pkg.${COMPRESSION_FORMAT} format. pkg-bootstrap should then be taught to try that first and fallback to pkg.txz if that doesn't work.

The .{pubkeysig,sig} will also need a symlink since it has the (and should probably retain the) compression format embedded in the name -- I think it makes sense to just create a pkg.signature symlink to simplify the fetching logic and switch on signature_type for verify_signature/verify_pubsignature.

I think this captured what bapt and I had discussed, at least -- questions / comments / concerns?

what about using pkg.bsd as the format-agnostic symlink name?

pkg.bsd -> pkg.txz

bapt suggests pkg.latest or pkg.bootstrap as the agnostic symlink name

on IRC @bapt suggests latest/pkg.bootstrap as the symlink, full URL pkg.freebsd.org/FreeBSD:13:amd64/latest/pkg.bootstrap
Note that this removes the Latest from the URL currently used for pkg.txz.

18:16 <@bapt> so all in all to be backward compatible you can just create a new symlink in bsd.port.mk
18:17 <@bapt> and coordinate with antoine so the hook so sign now also sign the pkg.bootstrap