Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: update to pydantic v2 #238

Open
wants to merge 60 commits into
base: opentrons-develop
Choose a base branch
from

Conversation

sfoster1
Copy link
Member

Contains the work to support pydantic v2 as in Opentrons/opentrons#14871 .

This is a lot because these are really rust packages and need modern package infrastructure, which we're cherry-picking out of upstream along with all their dependent commits. We can't just pull upstream because we don't want to update python.

jameshilliard and others added 30 commits December 6, 2024 15:33
Python has two build backends for packages that use Rust:
setuptools-rust and maturin. Both are provided by the pyo3 package
infrastructure (but that's not relevant for Buildroot).

The setuptools-rust build backend is a setuptools extension that is
capable of building python rust extensions.

The maturin build backend is a pep517 build extension that is itself
written in rust, it is itself bootstrapped using setuptools-rust but
is not itself a setuptools extension.

Both are from the pyo3 build infrastructure, so we add both of them in a
single patch. They also share a lot of the cargo-specific handling.

Signed-off-by: James Hilliard <[email protected]>
[Arnout: remove the _PYO3_ENV variables, the add little benefit]
Signed-off-by: Arnout Vandecappelle <[email protected]>
The python-maturin build backend while itself is a pep517 backend
uses setuptools-rust for bootstrapping purposes.

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Although pydantic-core likely implements a subset of the functionality
in pydantic 1.10.8 as we currently package, there will not be any
conflict as the modules namespace differ:

    import pydantic  # 1.10.8
    import pydantic_core

So, we can add pydantic-core, then bump pydantic; we don't need to do
both in the same commit.

Signed-off-by: James Hilliard <[email protected]>
[[email protected]: add explanations from James about no-conflict]
Signed-off-by: Yann E. MORIN <[email protected]>
Add new python-annotated-types runtime dependency.

Add new python-pydantic-core runtime dependency.

Migrate build backend from setuptools to hatchling.

License hash changed due to adding contributors:
pydantic/pydantic@7bc9c65

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Commit 6b915358babe introduced PKG_PYTHON_MATURIN_INSTALL_CMD while
it should be HOST_PKG_PYTHON_MATURIN_INSTALL_CMD.

Adding any new host python package using maturin setup type will
fail during the install step.

Fixes: 6b915358babe37a721b4f6e032ec884174bd6e0b

Cc: Arnout Vandecappelle <[email protected]>
Signed-off-by: Romain Naour <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
We can now significantly simplify the python-orjson build using the
new maturin setup type introduced in the python package
infrastructure.

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
We can now significantly simplify the python-rtoml build using the
new setuptools-rust setup type introduced in the python package
infrastructure.

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Migrate from setuptools-rust to maturin infrastructure.

Signed-off-by: James Hilliard <[email protected]>
We can now significantly simplify the python-cryptography build using
the new setuptools-rust setup type introduced in the python package
infrastructure.

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
The recent commits that touched vendoring and hashes, totally missed
the non-native vendored packages, like python packages that contain
rust code, and are thus cargo-vendored.

The issue in this case, is that we need to download the archive as it
is hosted and known to PyPI, but store it locally with our vendoring
suffix. This is inherently conflicting.

Fortunately, the PyPI webserver will ignore the query part of the URL,
so we can request the archive known to PyPI, and append an arbitrary
query, that is automatically constructed with the actual filename we
will use to store it. Basically, an URL for a python package like:
    https://pypi.org.pkg/pkg-hash/pkg-vesion.tar.gz
can be turned into:
    https://pypi.org.pkg/pkg-hash/pkg-vesion.tar.gz?buildroot-path=filename/python-pkg-version-cargo2.tar.gz

This way, we can use out default _SOURCE value, and construct a _SITE
that contains the actual package URL, with an arbtrary query.

NOTE: this is a stop-gap measure, to quickly fix those packages, while
waiting for a generic solution that works in all cases, not just with
PyPI.

NOTE-2: of course, if PyPI changes its policy, and no longer ignored the
query part, this is going to break again. Hence the need for a generic
solution...

Reported-by: Thomas Petazzoni <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
The package is not yet available on PyPI, so go fetch it at the source.

Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
pyproject.toml declared the tzdata package as a required dependency, but
this is in fact only needed on win32. This was fixed upstream [1]. Since
we don't actually check the runtime dependencies from pyproject.toml, we
don't need to do anything in Buildroot.

[1] pydantic/pydantic#10331

Signed-off-by: James Hilliard <[email protected]>
[Arnout: remove tzdata runtime dependency]
Signed-off-by: Arnout Vandecappelle <[email protected]>
While we're at it, also fix the _SOURCE variable so it contains the
-cargo2 suffix. Without this change, the downloaded tarball name
pre-vendoring is the same as post-vendoring and is thus overwritten.
This change was actually done in commit
c617ebbc977d28c5c0ebc7d3bdfa3888f432640c, but then forgotten when the
python-pydantic-core temporarily switched to upstream git instead of
PyPI.

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Link to Rust 1.68.0 announcement: https://blog.rust-lang.org/2023/03/09/Rust-1.68.0.html

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Link to Rust 1.68.2 announcement: https://blog.rust-lang.org/2023/03/28/Rust-1.68.2.html

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Link to Rust 1.70.0 announcement: https://blog.rust-lang.org/2023/06/01/Rust-1.70.0.html

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Link to Rust 1.70.0 announcement: https://blog.rust-lang.org/2023/07/13/Rust-1.71.0.html

Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
antocout and others added 25 commits December 9, 2024 18:01
Link to Rust 1.74.1 announcement:
https://blog.rust-lang.org/2023/12/07/Rust-1.74.1.html

The mips*-unknown-linux-gnu* targets were removed from
stage 2 with host tools in version 1.72.0.
Release note:
https://github.com/rust-lang/rust/releases/tag/1.72.0

Signed-off-by: Antoine Coutant <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
The bump to rust 1.74.1 [1] introduced a regression on host using
cmake < 3.20 since the llvm libraries used in rust compiler was
bumpted to llvm 17 [2] and now requires cmake >= 3.20 [3].

Select BR2_HOST_CMAKE_AT_LEAST_3_20 to build host-cmake when needed.

[1] https://gitlab.com/buildroot.org/buildroot/-/commit/05392a5eae61c2855bc8d94f5bf9677ebbc5462a
[2] rust-lang/rust@8c1c7d3
[3] llvm/llvm-project@cbaa359

Fixes:
https://gitlab.com/buildroot.org/buildroot/-/jobs/5880448635

Signed-off-by: Romain Naour <[email protected]>
Cc: Antoine Coutant <[email protected]>
Reviewed-by: Antoine Coutant <[email protected]>
Signed-off-by: Romain Naour <[email protected]>
Several rust tools are linking against zlib, so add the depedency
explicitly in HOST_RUST_DEPENDENCIES.

For now, host-rust build system is not able to find zlib provided
by Buildroot in HOST_DIR due to at least two issues that will be
fixed in followup commits.

Note that host-zlib is already in the dependency chain, by way of
host-openssl, but since rust needs for itself, we need to add it
as an explicit dependency.

Signed-off-by: Romain Naour <[email protected]>
[[email protected]: add not about transitive dependency]
Signed-off-by: Yann E. MORIN <[email protected]>
host-rust package depends on several host packages to provide tools and
libraries but it doesn't take into account out host libraries in
HOST_DIR while building rustc compiler. Indeed, rustc needs zlib and
fails to link if zlib is not installed on the host.

  error: could not compile `rustc_driver` (lib) due to previous error

If zlib is installed on the host, we can notice it with ldd tool (while
it should be linked with the one provided by Buildroot host-zlib):

  ldd [...]TestRust/host/bin/rustc
  libz.so.1 => /lib64/libz.so.1

Provide HOST_LDFLAGS using llvm.ldflags in config.toml.
(HOST_LDFLAGS provides -L$(HOST_DIR)/lib -Wl,-rpath,$(HOST_DIR)/lib)

With that fixed, rustc_driver link with libz from HOST_DIR but the
host-rust build still fail later due to another issue.

  error: could not compile `rustdoc-tool` (bin "rustdoc_tool_binary") due to previous error

Fixes:
https://gitlab.com/buildroot.org/buildroot/-/jobs/6256881545
http://autobuild.buildroot.org/results/a6b/a6b28783f29e6b729824bf42679a62f72ad5bee0

Signed-off-by: Romain Naour <[email protected]>
[[email protected]: slight rewording in commit log]
Signed-off-by: Yann E. MORIN <[email protected]>
While building the rust toolchain, the build system ends up using
cargo (from [...]/output/build/host-rust-bin-1.74.1/cargo/bin/cargo)
to build some tools like rustdoc-tool.

But the host-rust package doesn't use the cargo infractructure (since
it provides cargo binary) and our cargo environment variables [1] are
not set to crosscompile cargo packages in the rust toolchain.

For exemple, we usually set RUSTFLAGS="-C link-arg=-Wl,-rpath,$(HOST_DIR)/lib"
to force cargo using libraries provided by Buildroot in $(HOST_DIR)/lib.

RUSTFLAGS is actually needed to find zlib library (host-zlib) to link
rustdoc-tool when zlib is not installed on the host.

Add $(HOST_PKG_CARGO_ENV) in HOST_RUST_BUILD_CMDS since it already
includes RUSTFLAGS but also CARGO_HOME.

Fixes:

  error: could not compile `rustdoc-tool` (bin "rustdoc_tool_binary") due to previous error

[1] https://gitlab.com/buildroot.org/buildroot/-/blob/2024.02-rc1/package/pkg-cargo.mk#L165

Signed-off-by: Romain Naour <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
When we update the rust version, we need to update a large list of
hashes for rust-bin, and update the hash for rust (source). This is very
tedious and very error-prone.

Add a helper script that generates the hash files automatically, and
also iupdate the _VERSION in the .mk.

We decided not to carry the hint about the verification of the tarballs
against the upstream GPG signature for three reasons:

  - it requires that all the tarballs be downloaded, which can take
    quite some time;

  - the hash files are available for all the needed archives;

  - the hash files are downloaded over https, so if someone managed to
    get a hold of the rust server to provide backdoored archives, they
    can also change the hash files;

  - properly verifying the archives would require a chain of trust
    between the person running the upsate script, and the rust GPG key,
    which is not a given, and verifying sigantures using an unverified
    key is not providing much security, if at all.

Regenerate the hash files with that script.

Note (for the future, maybe): there are manifest files for each release,
https:/static.rust-lang.org/dist/channel-rust-VERSION.toml, that we
could use to generate the RUST_HOSTS and RUST_TARGETS list. However,
adding new hosts or new targets implies updating the corresponding
_ARCH_SUPPORTS and accompanying symbols, so better leave that to a
manual operation, at least for now.

Notes: Many thanks to James for providing an initial script with the
same purpose. Given the feedback from previous reviews, it was simpler
to rewrite it from scratch; it should now be much, much simpler.

Signed-off-by: Yann E. MORIN <[email protected]>
Cc: James Hilliard <[email protected]>
Tested-by: James Hilliard <[email protected]>
In rust 1.75.0, the follwong targets were dropped [0]:
    mips-unknown-linux-musl
    mips64-unknown-linux-muslabi64
    mips64el-unknown-linux-muslabi64
    mipsel-unknown-linux-musl

[0] rust-lang/rust@0387015

Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: Sebastien Laveze <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
When the rust package was added in Buildroot it was using
the 1.23.0 release [1]  where some vendored sources contained a
Cargo.toml.orig file that caused issues with
support/scripts/apply-patches.sh used by Buildroot package
infrastructure.

Since then, Buildroot is now using rust 1.74.1 (2024.02.x),
1.79.0 (2024.08.x) and 1.82.0 (master) where vendored
sources no longer contains any Cargo.toml.orig file.
So this post-extract hook is no longer needed.

Moreover, since Rust 1.81.0 are present in
src/tools/rustc-perf/collector/compile-benchmarks/<vendored pkg>
where there is no .cargo-checksum.json file.

Since then, the sed command fail and stop the build:

  sed: can't read [...]/host-rust-1.81.0/src/tools/rustc-perf/collector/compile-benchmarks/serde_derive-1.0.136/.cargo-checksum.json: No such file or directory

Fixes:
https://gitlab.com/buildroot.org/buildroot/-/jobs/8232367821

[1] b50522d

Signed-off-by: Romain Naour <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
pkg-cargo currently sets the --release flag unless BR2_ENABLE_DEBUG is
set. However, this does not accurately reflect the configured build
settings. In addition, it only works for packages that use the cargo
infrastructure directory and not with packages using the cargo
environment indirectly, such as pyo3 based python packages. To support
these, we really want to pass the necessary flags in PKG_CARGO_ENV.

In order to accurately reflect the configured build settings
(optimization and debug levels), we set the appropriate environment
variables according to the global settings.

There is no way to specify the profile to use through an environment
variable, it has to be set through a cargo flag like --release. Since we
can't easily control the profile flags used by non-cargo package
infrastructures, we instead set the env variables for both root profiles
(dev/release). For the aspects that are not affected by the global
settings (incremental, codegen-units, split-debuginfo), we set them
equal to the default for release - which in our context is the
appropriate choice even when BR2_ENABLE_DEBUG is set.

For reference the default cargo root profile settings are:
https://doc.rust-lang.org/cargo/reference/profiles.html#default-profiles

Cc: Moritz Bitsch <[email protected]>
Signed-off-by: James Hilliard <[email protected]>
Tested-by: Moritz Bitsch <[email protected]>
Tested-by: Adam Duskett <[email protected]>
Reviewed-by: Adam Duskett <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
While bumping kodi, we figured out that the kodi-texturepacker and
kodi-jsonschemabuilder were both re-downloading the main Kodi tarball,
even though they contain:

KODI_JSONSCHEMABUILDER_DL_SUBDIR = kodi
KODI_TEXTUREPACKER_DL_SUBDIR = kodi

Both are host packages, and turns out that changing those variables to
HOST_ ones made the download sharing work.

Commit efa7712 ("package/pkg-generic:
host variant inherits target download settings") introduced
inheritance of host variables from target variables from a number of
variables, including DL_SUBDIR. But it missed the fact that earlier in
pkg-generic.mk, the following line was defined:

  $(2)_DL_SUBDIR ?= $$($(2)_RAWNAME)

So, when this later code kicked in:

 ifndef $(2)_DL_SUBDIR
  ifdef $(3)_DL_SUBDIR
   $(2)_DL_SUBDIR = $$($(3)_DL_SUBDIR)
  endif
 endif

In fact it never did anything because $(2)_DL_SUBDIR would never be
undefined. This commit fixes this issue by properly adjusting the
logic to inherit the value of the target variable when it exists, or
defaulting to $$($(2)_RAWNAME) otherwise.

Cc: Yann E. MORIN <[email protected]>
Signed-off-by: Thomas Petazzoni <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Files in a git repository can be given attributes, like the usual eol
that can convert to-from crlf, cr, lf; those are applied when comitting
or checking-out a file.

There are also two attributes that are meant to be used when generating
an archive (with git archive): export-subst, and export-ignore, that
respectively substitutes format placeholders in a file, and excludes a
file from the archive.

Some package (e.g. pcm-tools, luajit) use the export-subst attribute
to generate versioning information. luajit, specifically, uses the UNIX
timestamp of the commit as the patch-level for its semantic versioning.

We don't use git-archive, because we need to get submodules and LFS
blob, which git-archive does not handle. So, our git backend tries to
impersonate git-archive as much as possible, but the support for git
attributes was lost when we converted it from using git-archive to
manually creating the tarball in 3abd5ba (support/download/git: do
not use git archive, handle it manually) in preparation for f109e7e
(support/download/git: add support for submodules) (arguably, a long
time ago...)

Extend the git backend to handle the export-subst attribute. There is
no git tool (that we could find) that does that automatically, except
git-archive, which we can't use; "git check-attr" however can report
whether a file has a specific attribute (and git check-attr can work
with \0-delimited fields and records).

So, we iterate over all the files in the repository, and filter those
that have the export-subst attribute set. Then for each file, we use a
bit of awk to do the replacement:

  - for each line (managed natively by awk), we iterate over each
    format placeholder,
  - for each placeholer, we query "git log" with the requested format,
  - we emit the replacement.

When doing the replacement, we decided to force abbreviating short
hashes to 40 chars, which is the length of a full sha1, rather than
actually abbreviating them:

  - letting git decide of the length is not reproducible over time:
    - as new commits are added, the short length will increase to avoid
      collisions,
    - newer git versions may decide on a different heuristic to shorten
      hashes,
    - users may have local settings with an arbitrary length (in their
      ~/.gitconfig for example);

  - deciding on our side of an "small" arbitrary value would not be
    viable long term either, as it might be too large to be minimum, or
    too short to avoid collisions.

The only reproducible solution is to use unabbreviated hashes.

Handling git-attributes also implies that the format of the generated
archives has changed, since we now expand placeholders, so we bump our
git format version.

Hash files for all git-downloaded packages will be updated in followup
commits.

Of all our git-downloaded packages, 5 are affected, and their hashes
will be updated in a followup commit too:

  - pcm-tools, which was known, and the one that triggered this commit;
    since we now expand placeholders, we can drop the post-extract hook;
    switching to a full hash in replacements also changes the hash of
    the generated archive;

  - qt5knx, qt5location, qt5mqtt, and qt5opcua: the file .tag at the
    repository root, contains only the full hash placeholder; that file
    is not used at all during the build (AFAICS);

Finally, a sixth package, luajit, uses export-subst; it currently relies
on the github-generated archive (because it happens to currently use a
format that is reproducible); it will also be converted in a follow-up
patch.

Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Woody Douglass <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Cc: Francois Perrad <[email protected]>
Cc: Arnout Vandecappelle (Essensium/Mind) <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
…ename

When we change the way we vendor packages, either because our download
backend or helpers evolve, or when the vendoring tools themselves change,
we must avoid generating new archives with the same name, or there would
be confusion when using older archives with newer Buildroot versions, or
the other way around (and that would mess with local caches, like the
one we share on s.b.o).

This is going to be the case for example, when we enforce a better and
more reproducible set of modes on archived files in the following
commits.

Introduce a version suffix for post-processed downloads, that we can
bump when needed.

Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Currently, when we generate archives, we rely on a few assumptions and
mechanisms to ensure reproducilibity. So far, we mostly accounted for
the content (i.e. content, filenames, and path) of the files we
archived, and this is OK (git and svn should provide reproducilbe
content by design, and cargo and go vendoring are also supposed to be
generating reproducible content.

However, tarballs do not only contain the content of the files; they
also have a few metadata about those files. Beyond filenames and paths,
which are already reproducible, there is the timestamp, the user and
group name and ID. Those are also accounted for and made reproducible.

The final touch (so far!) is that files have access rights (aka mode),
and those too are stored in tarballs. So far we accounted for those by
ensuring that Buildroot would always run under a known umask, thus
generating files with reproducible modes.

That falls short in one case that we did not envision, though: a shared
download directory, where extended attributes are set to provide a
default ACL that is permissive, to allow two or more users (with
different uid and gid) to all read and write to such a directory. This
is trivially achieved with something like:

    $ mkdir -p "${BR2_DL_DIR}"
    $ setfacl -m 'default:user::rwx' "${BR2_DL_DIR}"
    $ setfacl -m 'default:group::rwx' "${BR2_DL_DIR}"
    $ setfacl -m 'default:other::rwx' "${BR2_DL_DIR}"

This has the effect that:

  - files below BR2_DL_DIR are all set with user, group, and world read
    and write access,
  - files executable by the owner will also be group and world
    executable,
  - directories are user, group, and world readable, writable, and
    searchable.

This means that all the archives we generate from files in BR2_DL_DIR
will have modes that are different from those generated on other systems,
where only the traditional umask is used.

There are various solutions to solve that issue:

  - detect the situation and abort: that's not nice, because users have
    a legitimiate reason to want to share that directory,

  - find a solution for each affected download mechanism: git, svn, hg,
    cvs, bzr... and for each of the affected vendoring mechanism: go and
    cargo [0]; this is not nice, because it means a lot of repetition,
    with the risk that they diverge over time (e.g. one is fixed for a
    newer issue, while the others are left out due to an oversight...)

  - find a single, common solution that works in all cases, whatever the
    download mechanism and/or vendoring: this is the best, because we
    can extend and fix it once and everything else benefits from it.

We obviously go for the third option.

The common solution is rather simple. When creating the tarball in
support/download/helpers, give an option to tar to set the group and
other permissions to those of the user, but without write permission.

This implies that we must bump the version-suffix for the download
backends [1] and for the vendoring post-processes. It also implies that
the hash may change, under the following circumstances:

- Symlinks normally have permissions 0777 (because symlink permissions
  are in fact meaningless). They will now have permission 0755 in the
  tarball.
- If the original tarball (for vendored go and cargo packages) contained
  files that are readable or executable by owner but not by group or
  other, they will now be readable resp. executable by group and other
  too. Note that for writeable it is not the case, because those were
  already handled by our 0022 umask (which makes them not writeable by
  group and other).

Because the hash may change, we need to update the BR_FMT_VERSION for
everything that creates tarballs. Go and cargo didn't have one up to
now, the the previous commit added the possibility to give one. The ones
for git and svn have to be updated. Since it is now possible to have a
suffix for both the VCS and the post-processing, change the suffix to
something more descriptive than "-brX", i.e. -git3 for git, -go1 for
golang, etc.

The hash updates and filename changes will be handled in a follow-up
commit.

[0] Note however that the vendoring is currently not done in a
sub-directory of BR2_DL_DIR, but the cargo and go caches are located
there. Files that get copied from there to the vendoring area would be
tainted as well, and thus we want to address that situation as well.

[1] we currently do not have a CVS version suffix, because we do not
guarantee the reproducilibity of CVS archives (we can't); for hg, we are
currently using hg's own archive tool, and presumably that does not have
the mode issue because it is not using the checked-out files. Still,
doing the mode fix in a single location will help extend those two
backends in the future (if that ever happens...).

Reported-by: Peter Korsgaard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
We can't stay in the past forever and ever...

Since tar 1.35, the way some fields (devmajor and devminor) are stored
has changed. These fields exist for each file in the tarball, but only
used for device nodes. In previous versions of GNU tar, they were set to
zero; since 1.35, they are set to empty.

Although this doesn't change anything about the content of the tarball,
and it will be extracted in exactly the same way regardless of the tar
version used for extracting, it does change the hash of the tarball.
Therefore, we have to
- make sure that the correct version of tar is used;
- update the format version so that the filename is different from
  before.

Increment all BR_FMT_VERSION by one.

Require tar >= 1.35 instead of < 1.35.

Signed-off-by: Yann E. MORIN <[email protected]>
[Arnout: also increment BR_FMT_VERSION and extend the commit message]
Signed-off-by: Arnout Vandecappelle <[email protected]>
Currently, when we generate archives, e.g.  for git, svn, cargo or go, we
use the package _BASENAME_RAW as the root directory of the generated
archive.  For example, for package foo at version 1.2.3, that would generate
an archive rooted at foo-1.2.3/.

This is usually what we want, except in one specific condition: when the
package shares its download with another package *and* it is a generated
archive. In that case, the root directory will be different for each of
the two packages, which is incorrect, but was so far benign: we never
had any hash for such generated archives, and they were only generated
in two cases:
  - linux and linux-headers
  - barebox and barebox-aux

As we skip one directory depth when extracting the archives, we did not
care what the root directory was; whether it was that of one package or
the other was of no consequence.

But now that we can have hashes for archives generated from custom
versions, this breaks the usual case where the headers used for the
toolchains are those of the kernel to build for the target. In this
case, we may end up downloading the linux-headers package before we
download the linux package, so we'd get the hash for an archive rooted
at linux-headers-XXX/, but the one for the linux package the archive
would be rooted at linux-XXX/, or we may end up (e.g. with parallel
builds) downloading the linux package first and linux-headers next.

That would cause conflicts in hashes, as demonstrated by the only defconfig
we have in that situation, olimex_stmp157_olinuxino_lime_defconfig.

_BASENAME_RAW is a construct that is expanded to include the RAWNAME
followed by a dash and the version, if there is a version, or with just
the RAWNAME when there is no version.

We tweak the download macro to use _DL_SUBDIR followed by the version.

This is only used by VCS backends (cvs, git, svn...) and so there will
always be a version string, so no need to duplicate the case without a
version like is done for _BASENAME_RAW

_DL_SUBDIR defaults to _RAWNAME, so this is a noop by default, unless
the package declares it shares its download with another one, in which
case the generated archive will now be rooted as for the shared package.

This was triggered by:
    https://patchwork.ozlabs.org/project/buildroot/patch/[email protected]/

Reported-by: Peter Korsgaard <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Francois Perrad <[email protected]>
Signed-off-by: Peter Korsgaard <[email protected]>
Later commits will start using this variable.

Signed-off-by: Yann E. MORIN <[email protected]>
[Arnout: quote TAR="..."]
Signed-off-by: Arnout Vandecappelle <[email protected]>
The DOWNLOAD macro is always called in package context, so the PKG
variable is always set, and thus we do not need to pass the package
as a parameter.

Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Currently, we have no explicit, dedicated way to pass information to the
download post-process scripts, even though we do already need to pass
the path to the manifest when it is not the default, which we do with an
environment variable.

We'll soon need to be able to pass additional information to
post-process scripts, like instructing cargo to not use a lock file when
the package is not locked.

Extend the dl-wrapper with a new option with an argument, where the
argument will be passed as-is to the post-process script.

We explicitly do not document this new variable, as it is expected to
only be used in our package infrastructures (although there is currently
exactly one package that will directly need it, and we hope to drop that
in the future).

Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
Signed-off-by: Yann E. MORIN <[email protected]>
Cc: Thomas Petazzoni <[email protected]>
Signed-off-by: Arnout Vandecappelle <[email protected]>
…nload process

Root makefile imposes 'umask 0022', and this may be more restrictive than the
user's original umask - which could have provisions set to share files/dirs
with other users.
As an example, the imposed value makes the per-package download directories not
writeable for the group, but just for the owner - the user that issued the first
build that populated the per-package dl dir for the first time (say user A).
Thus, if a BR package changes its version (e.g. for buildroot update), and
another user (say user B, in the same group of A) starts a build, BR fails the
creation of package-xxx.tar.gz inside the dl dir, because user B has no write
permissions on that path. Furthermore, in the case of the git backend, this
makes the git cache not updatable by a different user. This is disruptive for a
host used by many users, all belonging to a certain group.

So, to allow sharing of a rw BR2_DL_DIR location among users, we save the
original umask value and restore it during the download process.

Signed-off-by: Luca Pesce <[email protected]>
[Arnout:
 - CURR_UMASK -> CUR_UMASK.
 - BR2_ORIG_UMASK -> BR_ORIG_UMASK.
 - Don't check if the umask is more permissive, apply it regardless. If
   the user explicitly doesn't want to make their DL_DIR readable by
   others, that's fine.
 - Don't export BR_ORIG_UMASK.
 - Only set BR_ORIG_UMASK if it we recurse, and only set umask if
   BR_ORIG_UMASK is set.
 - Add DOWNLOAD_SET_UMASK to simplify the latter.
]
Signed-off-by: Arnout Vandecappelle <[email protected]>
@sfoster1 sfoster1 marked this pull request as ready for review December 18, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants