Commit Graph

3381 Commits

Author SHA1 Message Date
Colin Walters
04839e200f Misc doc tweaks/cleanups
- Link to osbuild blog
- Drop CentOS 7 stuff
- clarify goals a bit
2021-02-12 16:45:46 -05:00
Colin Walters
35342dabca build-sys: Always run makecargo build
When working on a PR to add a sub-crate I hit the fact
that our `find` bit wasn't fully accurate and spent some
time debugging the fact that the code I got after `make`
wasn't up to date.

Since cargo is smart in general, let's stop trying to
second guess its dependencies and just run `cargo build`
every time `make` is run.

(I'm not sure why we didn't do this from the start)
2021-02-12 15:18:45 -05:00
Colin Walters
118178cef1 tree-wide: Fix C++ range iteration to use references
This is actually a perfect example of the tradeoffs involved
in our team of C/Rust programmers trying to use C++ so we
can use cxx-rs =)

It turns out that "for (auto foo : bar)" is really yet another
one of those C++ sharp edges just waiting to cut.  That
version does a copy of each value; one almost always wants to use
"for (auto & foo : bar)" which like Rust's `.iter()`.

In most of our code we were OK making copies, they were just
a performance hit, but in another place I was relying on the
"side effect" of `.c_str()` but that meant our value got
destroyed.
2021-02-12 12:50:02 -05:00
Kelvin Fan
84065c48df tests/common/libvm: Print error if bad tests/vmcheck/image.qcow2
Print error and exit if cannot find `tests/vmcheck/image.qcow2` or
bad symlink causing failure to spawn a VM when calling the
`vm_kola_spawn()` function.
2021-02-11 21:11:37 -05:00
Colin Walters
764de41cc6 Switch to using cxx-rs for treefile
This is one half of https://github.com/coreos/rpm-ostree/issues/2544
which aims to drop our use of `cbindgen`.
2021-02-11 15:44:38 -05:00
Colin Walters
094be6c469 rust: Add module doc headers
Specifically motivated by adding some docs in `treefile.rs` around
how to add a field, but I decided to just do a pass and
document everything at least a little.

View with e.g. `cargo doc --document-private-items`.
2021-02-11 14:16:38 -05:00
Colin Walters
20178bb25a tree-wide: Include GLib headers before libdnf
Alternative fix to https://github.com/rpm-software-management/libdnf/pull/1139
aka https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1935

This way libdnf's `extern "C"` over the glib headers doesn't apply
because we already processed that header.
2021-02-10 17:05:42 -05:00
Colin Walters
f065c0dd32 Add --enable-sanitizers (not on by default yet)
This way we at least get unit test coverage (which...
our unit test coverage doesn't do much because our
main code paths require privileges or virt).

One main blocker to this is that rustc doesn't expose
first-class support for this yet:
https://github.com/rust-lang/rust/issues/39699

At a practical level this works when building in release
mode but fails with `cargo test` for some reason; linker
arguments being pruned?  Not sure.

So I was able to use this when composing to find a bug,
but then for some other reason the client
side apparently infinite loops inside libsolv.

So we're not enabling this yet for those reasons, but
let's land the build infrastructure now.

```
(lldb) thread backtrace
* thread #4, name = 'pool-/usr/bin/r'
  * frame #0: 0x00007fd61b97200f libc.so.6`__memcpy_sse2_unaligned_erms + 623
    frame #1: 0x00007fd61cbc88e6 libasan.so.6`__asan::asan_realloc(void*, unsigned long, __sanitizer::BufferedStackTrace*) + 214
    frame #2: 0x00007fd61cc4b725 libasan.so.6`__interceptor_realloc + 245
    frame #3: 0x00007fd61baec43e libsolv.so.1`solv_realloc + 30
    frame #4: 0x00007fd61baf0414 libsolv.so.1`repodata_add_dirstr + 276
    frame #5: 0x00007fd61bb6f755 libsolvext.so.1`end_element + 53
    frame #6: 0x00007fd61b05855d libxml2.so.2`xmlParseEndTag1.constprop.0 + 317
    frame #7: 0x00007fd61b063548 libxml2.so.2`xmlParseTryOrFinish.isra.0 + 888
    frame #8: 0x00007fd61af7ed20 libxml2.so.2`xmlParseChunk + 560
    frame #9: 0x00007fd61bb727e7 libsolvext.so.1`solv_xmlparser_parse + 183
    frame #10: 0x00007fd61bb5ea0e libsolvext.so.1`repo_add_rpmmd + 254
    frame #11: 0x000055a4fce7a5f5 rpm-ostree`::load_filelists_cb(repo=<unavailable>, fp=<unavailable>) at dnf-sack.cpp:444:23
    frame #12: 0x000055a4fce7cad6 rpm-ostree`load_ext(_DnfSack*, libdnf::Repo*, _hy_repo_repodata, char const*, char const*, int (*)(s_Repo*, _IO_FILE*), _GError**) at dnf-sack.cpp:430:13
    frame #13: 0x000055a4fce7df60 rpm-ostree`dnf_sack_load_repo at dnf-sack.cpp:1789:26
    frame #14: 0x000055a4fce7eee9 rpm-ostree`dnf_sack_add_repo at dnf-sack.cpp:2217:28
    frame #15: 0x000055a4fce7f0fb rpm-ostree`dnf_sack_add_repos at dnf-sack.cpp:2271:32
    frame #16: 0x000055a4fce870ee rpm-ostree`dnf_context_setup_sack_with_flags at dnf-context.cpp:1796:29
    frame #17: 0x000055a4fcdf757f rpm-ostree`rpmostree_context_download_metadata at rpmostree-core.cxx:1206:44
    frame #18: 0x000055a4fcdf95c3 rpm-ostree`rpmostree_context_prepare at rpmostree-core.cxx:2001:48
    frame #19: 0x000055a4fce54ab7 rpm-ostree`rpmostree_sysroot_upgrader_prep_layering at rpmostree-sysroot-upgrader.cxx:1018:38
    frame #20: 0x000055a4fcdcb143 rpm-ostree`deploy_transaction_execute(_RpmostreedTransaction*, _GCancellable*, _GError**) at rpmostreed-transaction-types.cxx:1445:49
    frame #21: 0x000055a4fcdba4cd rpm-ostree`transaction_execute_thread(_GTask*, void*, void*, _GCancellable*) at rpmostreed-transaction.cxx:340:34
    frame #22: 0x00007fd61c58f7e2 libgio-2.0.so.0`g_task_thread_pool_thread + 114
    frame #23: 0x00007fd61c3d7e54 libglib-2.0.so.0`g_thread_pool_thread_proxy.lto_priv.0 + 116
    frame #24: 0x00007fd61c3d52b2 libglib-2.0.so.0`g_thread_proxy + 82
    frame #25: 0x00007fd61b8af3f9 libpthread.so.0`start_thread + 233
    frame #26: 0x00007fd61b9c9903 libc.so.6`__clone + 67
(lldb)
```
2021-02-10 14:30:37 -05:00
Colin Walters
e643ca8113 build: Drop introspection+clang hack
Forcing use of GCC breaks trying to use `clang -fsanitize=address`.
The real fix here anyways should have been avoiding passing
compiler flags to a compiler that doesn't understand them.
2021-02-10 14:30:37 -05:00
Jonathan Lebon
49fe650124 lockfile: Move ror_lockfile_read to cxx.rs
Pretty straightforward. Haven't moved `ror_lockfile_write` yet because
that's trickier to do and I'm still figuring out the most elegant way to
do this within cxx.rs' constraints.
2021-02-10 13:39:38 -05:00
Jonathan Lebon
b640892f04 libdnf-sys: Drop C API, replace with cxx.rs bridge
Right now, we're using libdnf APIs from Rust via hand-crafted `extern C`
interfaces, which is extra dangerous because there is no signature
checking that happens at compile-time.

Until either we can automate libdnf bindings or use its C++ API directly
via cxx.rs, let's do some basic wrapping in C++ ourselves and use libdnf
through that API only instead. That gives us a lot more confidence and
makes the libdnf API feel more natural to use in Rust.
2021-02-10 13:39:38 -05:00
dependabot[bot]
5cd9e8f5e8 build(deps): bump serde_yaml from 0.8.15 to 0.8.16
Bumps [serde_yaml](https://github.com/dtolnay/serde-yaml) from 0.8.15 to 0.8.16.
- [Release notes](https://github.com/dtolnay/serde-yaml/releases)
- [Commits](https://github.com/dtolnay/serde-yaml/compare/0.8.15...0.8.16)

Signed-off-by: dependabot[bot] <support@github.com>
2021-02-10 10:46:38 -05:00
Jonathan Lebon
56f95cc5c1 extensions: Don't try to chown RPM packages
We don't need it and it won't work in the unprivileged path where we're
running this in a cosa supermin (of course, this is all a bit silly
because we don't actually need privileges to begin with for this, but
there's a lot of momentum in sticking with that workflow).

Update submodule: libglnx
2021-02-10 10:20:38 -05:00
dependabot[bot]
e7c744e8b2 build(deps): bump serde_json from 1.0.61 to 1.0.62
Bumps [serde_json](https://github.com/serde-rs/json) from 1.0.61 to 1.0.62.
- [Release notes](https://github.com/serde-rs/json/releases)
- [Commits](https://github.com/serde-rs/json/compare/v1.0.61...v1.0.62)

Signed-off-by: dependabot[bot] <support@github.com>
2021-02-09 19:44:35 -05:00
Colin Walters
6886e44425 scripts: Bind /usr/share/empty over /usr/share/rpm
Now that we inject the `%_dbpath /usr/share/rpm` macro, `rpm -q`
will start using it.  But in RPM script invocation, we don't
want them to see any RPM database at all - trying to query it
should be a clean failure.
2021-02-09 18:36:35 -05:00
Jonathan Lebon
fb8e2da9d6 ci: Re-add CARGO_BUILD_JOBS
We lost this at some point during the CI re-shuffle. We need to
constrain cargo builds too to respect our CPU allocation.

This doesn't totally keep all jobs under 5 since e.g. we could have 5
make jobs and 5 cargo codegen builds going at once, but I think as long
as it's not something ridiculous like 40, it should be fine. Otherwise
we'll tighten it more.
2021-02-09 18:36:35 -05:00
Jonathan Lebon
667fdc9ff4 libpriv/rpm-util: Use /usr/share/rpm for base rpmdb query
Follow-up to previous commit: we had another path where we made a
temporary rootfs and symlinked `/var/lib/rpm` to the base rpmdb. That of
course broke now that we inject a macro to point the rpmdb to
`/usr/share/rpm`.

Rework this to use `/usr/share/rpm` since that's our canonical location
for now, but also add the compat symlinks so that this logic should keep
working even on trees without the injected macro yet.
2021-02-09 18:36:35 -05:00
Jonathan Lebon
60215ae865 libpriv/rpm-util: Add /usr/lib/sysimage/rpm symlink in rpmdb checkout
We don't technically need this yet, but it mirrors how it's set up in
our composes so that if there's code that wants to use the new location
too, it'll just work.
2021-02-09 18:36:35 -05:00
Jonathan Lebon
aa1a0f8719 lib/package: Fix comment about rpmdb checkout
We do fallback here.
2021-02-09 18:36:35 -05:00
Jonathan Lebon
99486a75e8 Add /usr/lib/rpm/macros.d/macros.rpm-ostree to set %_dbpath to /usr/share/rpm
We trigger a librpm macro file load in many of our paths. Since the
default value shipped by rpm's macro file sets `_dbpath` to
`/var/lib/rpm`, we have to explicitly set that back to `/usr/share/rpm`
in those paths.

This became more problematic recently with libsolv v0.7.17 which fully
keys off of `_dbpath` to find the rpmdb path to load:

04d4d036b2

And it's not technically wrong; we really should make that macro not
lie. This is what this patch does by injecting an RPM macro file in our
composes which sets it to /usr/share/rpm. So then e.g. the `rpm` CLI
doesn't actually need the `/var/lib/rpm` backcompat link anymore, though
there's no harm in leaving it.

In the future, we should be able to drop this once we move all of Fedora
to `/usr/lib/sysimage/rpm` (see
https://github.com/coreos/fedora-coreos-tracker/issues/639).

Closes: #2548
2021-02-09 18:36:35 -05:00
Jonathan Lebon
8ab604d098 ci: Temporarily use libsolv-0.7.17
We need to make sure that we can work with newer libsolv, which changed
how the rpmdb is found (see #2548).
2021-02-09 18:36:35 -05:00
Luca BRUNO
8a2f281143 libpriv/postprocess: get rid of goto statements
This removes all goto statements in the postprocess module,
replacing them with an exception catcher instead.
2021-02-09 16:43:32 +01:00
Colin Walters
2f6b5a654d Bind output core into Rust, use in apply-live
Originally the Rust apply-live code was exposed from Rust to C
via bindgen.  But when working on that, I hit the problem
that our output infrastructure was C...and the "reverse direction"
binding stuff was just ugly.

This PR again IMO shows the value of the investment in cxx-rs
because we can now seamlessly call back from the Rust side
into a "C++-ish" progress API, which the C++ side is updated
to use.

The level of indirection here is obviously pretty silly
because the main thing on the C++ output side is basically
a function dispatcher, but...I didn't want to try to rework
that into Rust fully yet.  (But, the moment we do this
whole area will get a *lot* cleaner)

Anyways, in the end this makes it easy for the apply-live
code to output progress to the user which was sorely
needed.
2021-02-09 04:43:29 -05:00
Colin Walters
c9e9269770 Rename internal Rust progress to console_
Our output system is very confusing in that we bridge over
DBus in some cases and not others.  In preparation for allowing
Rust code to call into the C++ progress system which contains
that delegation layer, rename the Rust progress to `console_`
to clearly show that it should only be invoked by code that
knows it's writing to a tty.
2021-02-09 04:43:29 -05:00
Colin Walters
8f71dcfafe build-sys: Statically link binary against shlib code
Having our binary depend on the shared library, which in
turn depends on the binary (at runtime) is messy.
Instead, statically compile the shlib code into our binary.
This duplicates the text a bit, but it's not a lot of code.

The goal is to more easily in the future to e.g. move the
shared library out into a separate git repository entirely
that runs on a separate lifecycle - that would still build
using Automake for example while the main git repository
switches to purely cargo.

Another motivation is avoiding linker issues I had with other
patches due to this semi-cyclical dependency.
2021-02-09 03:37:31 -05:00
Colin Walters
36566e406e build-sys: Add --enable-werror, rework compiler flags
So...at some point we somehow lost `-Wall` in our
default compiler flags which means we were missing some
potentially important warnings.  And
we used to have `-Werror` on in CI which combined
with the above was strongly opinionated about not
landing warnings in git master.

Our default stance here remains the same; we have
an opinionated set of `-Werror=` that applies in
*all* configurations.   However that set moves
into Automake - I don't think we need to do
compiler version detection anymore, we can assume
a modern compiler.

We also add back in `-Wall` by default now.

Further in CI, add `-Werror`.  The implementation
here is in our buildsystem rather than
`export CXXFLAGS=-Werror` because unfortunately
we have to fix things in libdnf too, and I don't
want to block entirely on that.
2021-02-08 16:50:09 -05:00
Colin Walters
a8fe6cf32b transaction: Fix missing return in error case
Hooray for compiler warnings.
2021-02-08 14:38:31 -05:00
Colin Walters
dba43e201e tree-wide: Fix clang -Wgnu-designator/-Wunused-variable
clang warns about this:
https://clang.llvm.org/docs/DiagnosticsReference.html#wgnu-designator
Also fix unused variables and hard error on them.
2021-02-08 14:38:31 -05:00
Colin Walters
b942bd8cf7 ci/clang-analyzer: Install deps
This flow was missing from here which broke in CI.
2021-02-08 13:21:16 -05:00
dependabot[bot]
4493fb00b7 build(deps): bump cxx from 1.0.29 to 1.0.30
Bumps [cxx](https://github.com/dtolnay/cxx) from 1.0.29 to 1.0.30.
- [Release notes](https://github.com/dtolnay/cxx/releases)
- [Commits](https://github.com/dtolnay/cxx/compare/1.0.29...1.0.30)

Signed-off-by: dependabot[bot] <support@github.com>
2021-02-08 12:07:16 -05:00
Colin Walters
8ddaf0bbd6 Make failure to find packages fatal, add more error prefixing
To help debug https://bugzilla.redhat.com/show_bug.cgi?id=1925584
2021-02-08 11:20:50 -05:00
Jonathan Lebon
d22d241754 core: Don't allow noent when resolving pkgcache rev
If we get here, it's that we expect the pkgcache to be there. So don't
allow ENOENT (we weren't even checking for the ENOENT case here, which
shows that this was the intent).

Related: https://bugzilla.redhat.com/show_bug.cgi?id=1925584
2021-02-08 11:20:50 -05:00
Colin Walters
5cefee81ff tree-wide: Squash some clang-analyzer found unused variables
We weren't using the return values, so just drop them.
2021-02-05 12:39:07 -05:00
Colin Walters
4d2a6e6de0 tree-wide: Pacify some clang-analyzer "Dead nested assignment"
It doesn't understand the "$x_owned" pattern (which is really
much like Rust's `std::borrow::Cow`).
2021-02-05 12:39:07 -05:00
Colin Walters
18c29fa5d6 libmain: Refactor to fix analyzer warning
Previously this function was in `goto out` style
and so we had this awkward "track return value in pointer"
thing.  But `clang-analyzer` correctly points out that
we don't need this anymore because we never read the value
initially stored.
2021-02-05 12:39:07 -05:00
Colin Walters
6a594dbe6b util: Annotate our "throw" wrappers as [[ noreturn ]]
This way the compiler and `clang-analyzer` understand and won't
issue an error if we are missing a `return` after an unconditional
`throw_gerror()`.
2021-02-05 12:39:07 -05:00
Colin Walters
a36f9716c6 tree-wide: Fix some "Dead assignment" from clang-analyzer
The `have_rpmdb` one was a leftover looks like.  In the `disabled_all_repos`
case it was clearly there for symmetry, but eh; it seems somewhat
unlikely that we add a *3rd* case there.  Also while we're
here change it to C++ `bool` so tools like analyzers know it really
is a boolean.
2021-02-05 12:39:07 -05:00
Colin Walters
7b5b35210b tree-wide: Fix some spurious "Dead assignment" from clang-analyzer
This fixes some spurious warnings from clang-analyzer (aka `scan-build`) around
"Dead assignment".  Unfortunately the analyzer doesn't understand
the side effects of `__attribute__((cleanup))` here.

More info on the `(void)` pattern: https://clang-analyzer.llvm.org/faq.html#dead_store
2021-02-05 12:39:07 -05:00
Colin Walters
6dd12a8175 ci: Add clang-analyzer.sh
There are really no excuses for any C/C++ project not to use
both ASAN+UBSAN and static analysis in CI.
2021-02-05 12:39:07 -05:00
Colin Walters
47c60eb6ce libdnf: Various buildsys fixes
WITH_SWDB: Removed in 99309fbe04
WITH_GIR Removed in e2f2862bed

Also, most importantly: don't always reconfigure libdnf

This is a questionable default for the cargo `cmake` crate.
Building in Koji is failing I think due to timestamp issues
causing cmake to run twice.
2021-02-04 17:57:33 -05:00
Jonathan Lebon
f5fde004dc Makefiles: Drop unnecessary cd rust/ and CARGO_TARGET_DIR
We can simplify these now that Rust is at the root dir.
2021-02-04 17:00:33 -05:00
Jonathan Lebon
df7adbb5f0 Makefile-rpm-ostree: Fix setting of cargo_target_dir
We don't want tabs here otherwise it doesn't work.
2021-02-04 17:00:33 -05:00
Jonathan Lebon
dd6b805fce Makefile-rpm-ostree: Drop hardcoded release path
And actually, we can just use `rpm-ostree` here, which symlinks to the
correct one (and `install` follows links).
2021-02-04 17:00:33 -05:00
Colin Walters
5060cdf7fc ci: Drop clang and unit tests from here
We're running them in Prow now and we're hitting capacity
issues in CentOS CI.  Bigger picture, the "just build and unit test"
stuff runs in any cluster, so let's save our bare metal capacity
for our compose/VM testing.
2021-02-04 15:19:36 -05:00
Luca BRUNO
1932979059 make/rust-test: fix linking and loading
This fixes link dependencies and build-libraries path, in order to
make Rust tests work.
It also introduces an additional wildcard target to allow specifying
a test filter to cargo.
2021-02-04 13:55:00 -05:00
Colin Walters
ae3392ffaf build-sys: Cleanly split up deps of public shlib vs internals
First, the public shared library only depends on a few
things (not the libdnf dependencies) so let's ensure we
only link it to those libraries.

And then, I realized we don't actually need the libdnf
dependencies here - I think I only added those back here
when trying vainly to keep the C unit tests working.  But
we don't have those anymore!  So we can delete the duplication
and fully rely on Cargo taking care of libdnf.

Conceptually for a static library we don't "link" it against
anything in Automake, that happens at the final stage with
the Rust linker.
2021-02-04 10:59:20 -05:00
Colin Walters
19c7fbb7c9 build-sys: Delete duplicate Rust pkg-config dependencies
We didn't need this after switching to a Rust main.
2021-02-04 10:59:20 -05:00
Colin Walters
c383c9511f build-sys: Drop Fedora 25+ rpm version check
I think we can just assume this nowadays.
2021-02-04 10:59:20 -05:00
Colin Walters
588541c60d Move libdnf build over to Cargo
This is now further migration towards Cargo/Rust possible
because we switched our main binary.  We've had an internal
`libdnf-sys` crate for a while, but now it can take over
the build of the underlying library too (like many `-sys`
crates support).

This itself is just an incremental step towards migrating
the main rpm-ostree build system to e.g. cmake too (or
perhaps directly with the `cc` crate, not sure yet) and
driving it via `cargo` too.
2021-02-04 10:59:20 -05:00
Colin Walters
2128f5784a build-sys: Explicit workspace, move libdnf deps to crate
First explicitly state that we're a workspace.  AIUI
this is actually implicit today via our use of a `path`
dependency, but in the future we may have other sub-crates.
So let's make it explicit now.

Also move the libdnf dependencies directly to that sub-crate.
2021-02-04 10:59:20 -05:00