This is what we do for non-local (i.e. HTTP) pulls; we wnat to
correctly handle being interrupted during partial pulls.
Closes: https://github.com/ostreedev/ostree/issues/579Closes: #613
Approved by: jlebon
This is a follow up to conversation on list - in practice, if we're
backing away from summary signing, then it makes sense to remove the
special casing for checksums in deltas around summary signatures.
This is also related to the recent change to enable GPG checking for
commits in deltas - now we have a more coherent story between the
previous pull path and deltas.
I didn't do any performance checking, and while it's slightly annoying
that we're now doing sha256 on the delta content twice (once for the
part and once per object)...sha256 is pretty fast, I think most users
are I/O bound anyways, and it'd drop even farther if we started using
openssl.
Closes: #612
Approved by: jlebon
The spam in stderr was bothering me, and further at some eventual
point in the future we want to annotate the functions with
`__attribute__((nonnull))` which would then cause tests like these to
become undefined behavior.
The coverage of this isn't worth the log spam basically.
Closes: #611
Approved by: jlebon
I am trying to track down a warning I see in `test-keyfile-utils`
which turned out to be the installed case only, but let's inject
this here too.
(The GLib default is broken, but it's hard to fix upstream without
breaking the world)
Closes: #610
Approved by: jlebon
We should be religious about the "only set output variables on
success", otherwise it makes leaks more likely.
But the real leak was us simply not using autoptr in one place.
Closes: #598
Approved by: jlebon
And "move semantics" via `g_steal_pointer()`. Just a minor code
cleanup I noticed when I was hunting for a leak, which ended up being
elsewhere.
Closes: #598
Approved by: jlebon
I find myself often wanting to debug interactively failing tests.
This makes it more convenient to keep around the temporary directories
just for those tests, rather than accumulating tons of tempdirs from
the successful tests as well.
Closes: #588
Approved by: jlebon
The fact that we weren't doing this is at best an oversight, and
for some deployment models a security vulnerability. Having both
`gpg-verify` and `gpg-verify-summary` shows that we were intending
them to be orthogonal/independent.
Lately I've been advocating moving towards pinned TLS instead of
gpg-signed summaries, and if we follow that path, performing GPG
verification of commit objects even if using deltas is more important,
as it provides an at-rest verifiable authenticity and integrity
mechanism.
Content providers which are signing their summary files and/or using
TLS (particularly pinned TLS) for transport should treat this as a
nice-to-have. However, for providers which are serving content over
plain HTTP and relying on GPG, this is a critical update.
Closes: https://github.com/ostreedev/ostree/issues/517Closes: #589
Approved by: jlebon
`-fsanitize=address` complained that the `refcount > 0` assertions
were reading without atomics. We can fix this by reworking them
to read the previous value.
Closes: #582
Approved by: jlebon
It turns out this is basically racy with the presence of other
threads. It was really cosmetic so let's stop doing it and make
`-fsanitize=thread` happy.
Closes: #582
Approved by: jlebon
This is actually fine in practice, but it triggers this
`-fsanitize=undefined` warning I saw in the test suite log:
```
src/libostree/ostree-repo-static-delta-compilation.c:160:10: runtime error: null pointer passed as argument 1, which is declared to never be null
```
Closes: #584
Approved by: jlebon
Conceptually we've been moving towards having our GPG verification
paths be per-remote. The code internally supports this, but we
didn't expose an API to use it conveniently.
This came up when trying to add a new `gpgkeypath` option, since
right now rpm-ostree manually finds keyrings for the remote, and
hence it wasn't looking at the keypath, and said "Unknown key"
in status.
Adding an API fixes this nicely.
Closes: #576
Approved by: giuseppe
For Project Atomic, we already have RPM signatures which use files in
`/etc/pki/rpm-gpg`. It's convenient to simply bind the OSTree remote
configuration to those file paths, rather than having duplicate key
data.
This does mean that we need to parse the files for verification, so we
end up importing them into the verifier's temporary keyring, which is
a bit ugly, but it's what other projects do.
Closes: https://github.com/ostreedev/ostree/issues/573Closes: #575
Approved by: giuseppe
What in the code is called "scanning" is ensuring (potentially
recursively) have an object, and if not, fetching it. And then if
it's metadata, parsing it and finding new objects to fetch.
This logic has grown fairly complex. What I'm trying to fix
right now is that if we're doing a pull-local to a remote repository
via `sshfs` (FUSE) we still end up scanning, which is inefficient.
We can take advantage of the "commitpartial" logic here - if a commit
isn't partial, it's complete, hence we don't need to scan it.
At the same time, I'm changing the logic here to *always* do scans for
dirtree objects. This will fix cases where multiple commits share
dirtree objects. We have "commitpartial" metadata, but no such concept
of partial/complete for dirtrees.
But, we'll only ever scan dirtrees if we scan commits, which is
what the section above fixes.
Closes: https://github.com/ostreedev/ostree/issues/543Closes: #564
Approved by: alexlarsson
Some deployments may want to gate access to content based on things
like OAuth. In this model, the client system would normally compute a
token and pass it to the server via an API.
We could theoretically support this in the remote config too, but
that'd be a bit weird for OAuth as the information is dynamic.
Therefore this cleans up the code a little bit to more clearly handle
the case that the fetcher is initialized from both remote config
data plus pull options.
Closes: #574
Approved by: giuseppe
I don't know what's going on, I suspect mirror churn. Anyways,
it seems to be consistently failing now, so let's drop it.
Closes: #571
Approved by: jlebon
Otherwise it's possible for us to exhaust available file descriptors
or (on 32 bit) run up against mmap limits.
In the rollsum case, we didn't need to hold open the "from" object
at all. And in the bsdiff case, we weren't even looking at either of
the files until we started processing.
Also, while we have the patient open, switch to using O_TMPFILE
if available.
Closes: #567
Approved by: giuseppe
We should just download the commit objects directly, as it's
obviously a lot more efficient than deltas.
I had to generate a summary file in more places in the tests,
since once created, it needs to be updated.
Closes: https://github.com/ostreedev/ostree/issues/528Closes: #566
Approved by: jlebon
I was doing a chain of mirroring like A -> B -> C
And repo B had A as a remote. When I added B as
a remote to C, the summary file of B had a ref
upstream:foo/bar/baz, which caused all pulls from
B to C to fail, since the summary file is only
expected to have refs, not refspecs.
Closes: https://github.com/ostreedev/ostree/issues/561Closes: #565
Approved by: jlebon
I'm doing builds and `make check` inside a Docker container, with
selinux on as a build-time option, but no policy in the container.
This currently aborts. Let's not do that.
(This type of thing is why installed tests are a better model)
Closes: #546
Approved by: jlebon
I was noticing a recent performance issue with checkouts
which seemed to be mostly us going back to doing a `fsync()` on
directories.
Regardless, while looking at that, I saw we were spending time
creating new symlinks. Even though symlinks are small, it's still
better to hardlink them.
Going way back in time, the reason we weren't doing this is
because we were hitting `EMFILE` on ext4, but that was for
gnome-continuous which creates *many* build roots. Even
there though, they're just a cache, and we handle `EMFILE`.
For ostree-for-host-system, we don't expect to have many roots (just 3
at most transiently), so hardlinking symlinks does make sense.
Closes: #521
Approved by: jlebon
This pulls in a new compilation flag for wrpseudo compatibility. Also
note we need to add some includes since glnx-libcontainer went away,
and with it some includes for `sys/mount.h` etc.
Closes: #522
Approved by: cgwalters
We were seeing some weird potential memory corruption in this code
when using it for `rpm-ostree-toolbox installer`, which is almost
certainly not its fault, but let's use it as an excuse to port
(mostly) to fd-relative and away from GFile.
Dropping the last GFile use here is a bit tricky as it does have a
nice high level wrapper around inotify.
Closes: #512
Approved by: jlebon
My previous change in https://github.com/ostreedev/ostree/pull/425
actually broke things so we basically used the repository defaults =(
This is a subtle mess since we're only trying to flip things off
for the *new* API.
Clean this up so that the "default repo inheritance" lives only in one
place - in the compat layer for the old checkout API. The new
checkout API defaults to off period, so the repository state is
irrelevant.
Closes: #520
Approved by: jlebon
While looking at a slow update issue (which I'm guessing is
unpredictable I/O latency in an OpenStack instance), I noticed
in one of the traces we were inside a fsync here.
Dropping the fsync here is just another of a long series of unwinding
them - we `syncfs()` the sysroot fd and `/boot` and we have a big
`sync()` anyways.
Closes: #508
Approved by: jlebon
More fsync pruning. Since we have a public API for writing the origin
file and it did a fsync before, let's preserve that. But when writing
deployments as part of a full transaction, we rely on the global
`syncfs()`, so add an internal function for origin file writing that
doesn't.
Closes: #509
Approved by: giuseppe
In https://github.com/ostreedev/ostree/pull/408 we fixed a
bug where we would crash when trying to execute deltas into
an archive repo (which isn't presently supported).
But that was overly aggressive - we obviously *can* execute deltas
when mirroring into a bare repo. This should fix a regression with
the way flatpak uses mirroring to pull from a user repo into the
system.
Closes: #506
Approved by: alexlarsson
This way e.g. flatpak can detect which timestamp it should use.
See `flatpak/common/flatpak-utils.c:flatpak_zero_mtime`.
Closes: #501
Approved by: cgwalters
Since forever, we've been doing two cleanups. In
https://github.com/ostreedev/ostree/commit/8ece4d6d51bdbe3e41ab318259276bb83e553aa0
I thought we were doing just one and wanted to go to zero (if specified),
but I actually just dropped one cleanup.
In https://github.com/projectatomic/rpm-ostree/pull/452
@jlebon pointed out the duplication. Fix this by creating a new internal
deploy wrapper that takes cleanup flags.
(Since we already had the "piecemeal cleanup" API internally, let's
frame it in terms of that, rather than passing down a boolean).
Closes: #500
Approved by: jlebon
If the current repo is already up to date (we have no content to
fetch), it's possible for the fetcher to not request any URIs. So
create and then finalize it quickly.
Finalization involves calling `g_main_loop_quit()` +
`g_thread_wait()`. However, if `g_main_loop_quit()` is run *before*
`g_main_loop_run()`, we'll deadlock because GMainLoop assumes in
`_run()` to start things.
This is a common trap - ideally, GMainLoop would record if `_quit()`
was called before `_run()` or something, but doing that now would
likely break people who are expecting quit() -> run() to restart.
In general, we've moved in various GLib-consuming apps to an
explicit "main context iteration with termination condition" model;
see `pull_termination_condition()` in the pull code.
This fixes this race condition.
I verified that an assertion in `_finalize` that more than
zero URIs were requested was hit in multiple test cases, and this patch
has survived a while of make check loops.
Closes: https://github.com/ostreedev/ostree/issues/496Closes: #499
Approved by: jlebon
This should help avoid address space exhaustion on 32 bit systems,
and in general is obviously going to improve efficiency.
Closes: #498
Approved by: alexlarsson
Quoting Dan Nicholson in
<https://github.com/ostreedev/ostree/pull/330#issuecomment-245499099>
mtime of 0 has been the semantics of ostree deployments from basically
the beginning of the project. We (and others, see
flatpak/flatpak@b5204c9) rely on that fact when generating trees.
In particular, this affects caches that use the mtime of the
associated file or directory to determine if the cache is valid. By
arbitrarily changing the mtime of the files to something else, all
the caches we setup in the build are now invalidated. Preseeding
caches is really important to the user experience as it avoids
having the user wait while they're regenerated on first run.
Now, we could change our build infrastructure to preset all the
mtimes to 1 to match this change, but what does that do for our
existing users who are on an ostree that deploys with mtimes of 0?
We could just revert this change at Endless (and the associated one
in Flatpak), and that would be fine for our users. However, if we
point non-Endless users to our apps, they'll have the great
experience of waiting 10 seconds the first time they launch it while
the fontconfig cache is rebuilt unnecessarily.
Closes: #495
Approved by: jlebon
We hold a fd open on this, and it's basically now expected
to be immortal. Confer that status.
This was showing up in flatpak crashers, because we'd get
an unexpected errno.
(I didn't test this fixes the crasher, but it's clearly right)
https://bugzilla.redhat.com/show_bug.cgi?id=1347293Closes: #476
Approved by: alexlarsson
I noticed seeing the output of `prune` twice in rpm-ostree, and had
always wondered why. When reading the rpm-ostree code to fix
something else, reasons, I noticed the reason - we were pruning once
here, and then once after rpm-ostree regenerates its "base" refs.
There's no reason to clean twice, so let's add a flag so rpm-ostree
can suppress doing it inside libostree.
Closes: #474
Approved by: giuseppe
It's close-on-exec, not close-on-fork. I was clearly confused when
writing this; it works just fine to reference the fd in the child and
`fchdir()` before exec. So drop the unnecessary duplication.
Just noticed this while reading the code for a random other reason.
Closes: #473
Approved by: giuseppe
When doing a prune, we should not try to delete objects in parent
repos, since it'll fail. There is a bigger discussion about the
semantics of `parent=` to be had, but this will fix trying to use
`ostree prune --repo=/ostree/repo/extensions/rpmostree/pkgcache`.
Closes: https://github.com/ostreedev/ostree/issues/467Closes: #471
Approved by: jlebon
In CentOS, these happened to appear in a repo that is served
via rsync, and having them not be world-readable caused mirroring
tools to fail.
They aren't secret, so don't make them so.
Closes: #468
Approved by: giuseppe
Programs like `useradd` try to `open(/etc/passwd, O_RDWR)` to append,
which didn't work with rofiles-fuse. Thinking about this, I realized
that there's a simpler algorithm for "can we write to this file" which
is "does it have a hardlink count <= 1"?
Switching to this both drops complexity (we no longer need to keep a
hash table of files we created), and also lets useradd work.
Closes: #462
Approved by: jlebon
The test suite was actually doing something before, just
not quite what I intended. Without `-U` for bare-user checkouts
we end up doing a copy.
Now, a future commit will change how rofiles work, which would cause
the test suite to permit inplace mutation for non-hardlinked files.
So let's ensure they *are* hardlinked.
Closes: #462
Approved by: jlebon
Lots and lots of preparation led to this moment - when nothing
apparent changes for users! Woo!
But seriously, having the extra dependency is a minor annoyance, and
in the big picture I think the libgsystem idea was wrong - we need to
land things in GLib, and use git submodules for API-unstable or
Linux-specific sharing. For a lot of OSTree, the libgsystem `GFile*`
orientation was also wrong, we really want fd-relative.
Closes: #444
Approved by: jlebon
I hit an error with [CAHC](https://wiki.centos.org/SpecialInterestGroup/Atomic/Devel)
where we were doing time-based pruning. `ostree summary -u` started failing,
and it took me a bit to realize it was because we were pruning
even the tip of old branches, which I was not at all expecting,
and I don't think users will too.
Perhaps in the future we could add some sort of --prune-ref-tips or
something if people wanted it, but I doubt it.
Closes: #445
Approved by: jlebon
Since we already had a "recursive copy" implementation here, let's
reuse it rather than the libgsystem `gs_shutil_cp_a()`. Part of the
libglnx porting.
Closes: #428
Approved by: jlebon
It handles ownership of the `DIR*` for us more cleanly, and
is just a better API.
This is in preparation for further changes to this code to do SELinux
labeling while copying.
Closes: #428
Approved by: jlebon
I want to add `-Werror=int-conversion`, but it's only available in
newer GCC versions. So let's start autodetecting available compiler
flags.
Closes: #431
Approved by: giuseppe
I forgot to actually remove `config_file` in the previous
commit, the txn lock hasn't been used in a long time, and
for the uncompressed cache, everything uses the fd already.
Closes: #433
Approved by: giuseppe
I think we'd end up closing stdin...yuck. Need to investigate
having this be fatal, but CentOS 7 `gcc-4.8.5` doesn't understand
`-Werror=int-conversion`.
Closes: #430
Approved by: giuseppe
We have a better API now, drop use of the internal helper, which also
depended on libgsystem.
This required bumping libglnx to pull in a fix.
Closes: #429
Approved by: giuseppe
Since this is a new API, and adding booleans is the most likely thing
we'll do, let's stick some explicit padding for them in here now.
We could use the `unused_ints[]` but it'd be out of order, and this
will more clearly remind people about the padding. The efficiency hit
versus bitfields is annoying, but oh well, not a real world problem.
Closes: #427
Approved by: giuseppe
Colin Walters