I now think commit fab1e113db was a mistake;
because it breaks the mental model that at least I'd built up that "local repos
don't have checksums verified, HTTP does".
For example, a problem with this is (with that mental model in place) it's easy
for people who set up mirrors like this to then do local pulls, and at that
point we've done a deployment with no checksum verification.
Further, since then we did PR #671 AKA commit 3d38f03 which is really most of
the speed hit.
So let's switch the default even for this case to doing checksum verification,
and add `ostree pull --http-trusted`. People who are in situations where they
know they want this can find it and turn it on.
Closes: https://github.com/ostreedev/ostree/issues/1211Closes: #1212
Approved by: jlebon
Rather than carrying two booleans, just convert `OstreeRepoPullFlags`
into `OstreeRepoImportFlags`. This allows us to drop an internal
wrapper function and just directly call `_ostree_repo_import_object()`.
This though reveals that our mirroring import path doesn't check the
`OSTREE_REPO_PULL_FLAGS_UNTRUSTED` flag...it probably should.
Prep for further work.
Closes: #1212
Approved by: jlebon
Make the "local repo" processing conditional the same as the "localcache" bits;
this is really just a de-indent. Also add some comments. Prep for further work.
Closes: #1212
Approved by: jlebon
Noticed this while reading the code. The `child` var hasn't been
initialized yet at the time we throw this error (and even then, it's
only conditionally initialized). To be nice, let's just always calculate
the child path and pass that along.
Also do some minor style porting to decl near use.
Closes: #1216
Approved by: cgwalters
Add a few comments for each of the central functions used for committing
data from a directory. Took me a bit to understand the relationship
between those functions.
Closes: #1216
Approved by: cgwalters
This fixes up the last of the embarassing bits I saw from
the stack trace in:
https://github.com/ostreedev/ostree/issues/1184
We had a hardlink fast path, but that doesn't apply across
devices, which occurs in two notable cases:
- Installer ISO with local repo
- Tools like pungi that copy the repo to a local snapshot
Obviously there are a lot of subtleties here around things like the
bare-user-only conversions as well as exactly what data we copy. I think to get
better test coverage we may want to add `pull-local --no-hardlink` or so.
Closes: #1197
Approved by: jlebon
For the old `OSTREE_REPO_MODE_ARCHIVE_Z2`. Use it mostly tree
wide except for the repo finder tests (to avoid conflicting with
some outstanding PRs).
Just noted another user coming in some of those tests and wanted to do a
cleanup.
Closes: #1209
Approved by: jlebon
We added a `.dir-locals.el` in commit: 9a77017d87
There's no need to have it per-file, with that people might think
to add other editors, which is the wrong direction.
Closes: #1206
Approved by: jlebon
Add a hash function for OstreeRepo instances, which relies on the repo
being open, and hence being able to hash the device and inode of its
root directory.
Add unit tests for this and ostree_repo_equal().
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://github.com/ostreedev/ostree/issues/1191Closes: #1205
Approved by: cgwalters
Such an evil bug 🙈. I was just reading an strace trying to figure out what was
going on, and noticed we had the `XXXXXX` in the lockfile name. It was only
after that I realized that that this might *be* the cause of the skopeo issue.
This is another case where we definitely need more test coverage of things that
actually use the API multiple times in process; might look at dusting off the
work for the rpm-ostree test.
Closes: https://github.com/ostreedev/ostree/issues/1196Closes: #1204
Approved by: jlebon
While opening a repo we've recorded the device/inode for a while; use it to
avoid calling `linkat()` during object import if we know it's going to fail.
Closes: #1193
Approved by: jlebon
Conceptually `ostree-repo-pull.c` should be be written using
just public APIs; we theoretically support building without HTTP
for people who just want to use the object store portion and
do their own fetching.
We have some nontrivial behaviors in the pull layer though; one
of those is the "bareuseronly" verification. Make a new internal
API that accepts flags, move it into `commit.c`. This
is prep for further work in changing object import to support
reflinks.
Closes: #1193
Approved by: jlebon
There are use cases for not syncing at all; think build cache repos, etc. Let's
be consistent here and make sure if fsync is disabled we do no sync at all.
I chose this opportunity to add tests using the shiny new strace fault
injection. I can forsee using this for a lot more things, so I made
the support for detecting things generic.
Related: https://github.com/ostreedev/ostree/issues/1184Closes: #1186
Approved by: jlebon
Update the comments and remove an unneeded variable to make it clear
that the find_remotes_async() / pull_from_remotes_async() functions use
the unsigned summary support.
This is a follow-up of commit 8c148eb7e "lib/repo-finder: Emit
gpg-verify-summary=false in dynamic remote config".
Closes: #1195
Approved by: pwithnall
I saw in a stack trace that the main thread was calling `exit()` even while
worker threads were alive and doing sha256/write/fsync etc. for objects.
The stack trace was a SEGV as the main thread was calling into library
`atexit()` handlers and we were a liblz4 destructor:
```
#0 0x00007f2db790f8d4 _fini (liblz4.so.1)
#1 0x00007f2dbbae1c68 __run_exit_handlers (libc.so.6)
```
(Why that library has a destructor I don't know offhand, can't find
it in the source in a quick look)
Anyways, global library destructors and worker threads continuing simply don't
mix. Let's wait for our outstanding operations before we exit. This is also a
good idea for projects using libostree as a shared library, as we don't want
worker threads outliving operations.
Our existing pull corruption tests exercise coverage here.
I added a new `caught-error` status boolean to the progress API, and use it the
commandline to tell the user that we're waiting for outstanding ops.
Closes: #1185
Approved by: jlebon
We have a lot of layers of abstraction here; let's fold in the `trusted`
conditional into the call, since that's all the public API we're using does
anyways.
Prep for a future patch around object copying during imports.
Closes: #1187
Approved by: jlebon
This was some incomplete planning from while the find_remotes() API was
being designed; now totally outdated.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1179
Approved by: cgwalters
See issue #1174 for the rationale behind this. In summary:
• It required two lists of collection–refs to be maintained: one in the
repository, and one pointing to the repository.
• It didn’t automatically work for live USBs of OSs based on OSTree
(where there’s always a repository at /ostree/repo).
• It was unnecessarily complex.
The new scheme allows a list of repositories to be searched, but without
needing a layer of indirection through their collection–refs. It adds
/ostree/repo and /.ostree/repo as well-known repository locations which
are always checked on a mounted volume (if they exist).
Update the unit tests accordingly.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
https://github.com/ostreedev/ostree/issues/1174Closes: #1179
Approved by: cgwalters
This will compare their root directory inodes to see if they are the
same repository on disk. A convenience method for the users of the
public API who can’t access OstreeRepo.inode.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1179
Approved by: cgwalters
Update libglnx, which is mostly port the repo stagedir code
to the new tmpdir API. This turned out to require some
libglnx changes to support de-allocating the tmpdir ref while
still maintaining the on-disk dir.
Update submodule: libglnx
Closes: #1172
Approved by: jlebon
Doing this in prep for libglnx tmpdir porting, but I think we should also do
this because the partial fetch code IMO was never fully baked; among other
things it was never integrated into the scheme we came up with for "boot id
sync" that we use for complete/staged objects.
There's a lot of complexity here that while we have some coverage for, I think
we need to refocus on the core functionality. The libcurl backend doesn't have
an equivalent to this today.
In particular for small objects, this is simply overly complex. The downside is
clearly for large objects like FAH's 61MB initramfs; not being able to resume
fetches of those is unfortunate.
In practice though, I think most people should be using deltas, and we need to
make sure deltas work for large objects anyways.
Further ultimately the peer-to-peer work should help a lot for people
with truly unreliable connections.
Closes: #1176
Approved by: jlebon
I was looking at fixing an `rpm-ostree livefs` bug where we need to replace
`/usr/lib/passwd`. It's obviously bad if that temporarily disappears 😉. My plan
is to do a subpath checkout of just `/usr/lib/{passwd,group}`.
Make this atomic (i.e. file always exists) by changing the logic to create a
temporary link in repo/tmp, then rename() it into place.
A bonus here is we kill one of the very few (only?) non-error-cleanup i.e.
"non-linear" `goto`s in the ostree codebase.
Closes: #1171
Approved by: jlebon
It turns out that librpm automatically merges identical files between
distinct packages, and this occurs in practice with Fedora today between
`chkconfig` and `initscripts` for exmaple.
Since we added this for rpm-ostree, we basically want to do what librpm does,
let's change the semantics to do a merge. While we're here rename
to `UNION_IDENTICAL`.
Closes: #1156
Approved by: jlebon
If the new configuration passed to ostree_write_config () tries to
update options for a remote defined in a separate config file, return an
error. Without this, the full configuration would contain duplicate
remote specifications, which would raise an error the next time the repo
is opened.
Closes: #1159
Approved by: cgwalters
The new libglnx `glnx_mkdtempat()` uses autocleanups, which
is inconvenient for this use case where we *don't* want autocleanups.
Since we don't need it to be fd-relative, just directly invoke
`g_mkdtemp_full()` which is fine for this use case.
Prep for updating libglnx.
Closes: #1161
Approved by: jlebon
This option allows a repo to explicitly opt out of adding new remotes in
a remotes configuration directory. This currently defaults to true for
system repos and false for non-system repos to maintain legacy behavior
that non-system repos don't add remotes in a configuration directory.
That would be problematic for flatpak, which specifies a remotes config
dir but adds remotes in ways that are incompatible with it.
So, what this really does is allow system repos to control whether they
want to add remotes in the config dir or not. That's important if your
flatpak repo is the system repo like at Endless.
Closes: #1134Closes: #1155
Approved by: cgwalters
There was only one tricky bit here around the ownership of the lines; I made use
of `g_steal_pointer()` to consistently track ownership, and converted to a `for`
loop while still preserving the loop logic around the last entry.
Closes: #1154
Approved by: jlebon
Steal some code from flatpak for this, which allows porting a few more things to
new style. I started on a public API version of this but was trying to roll some
other things into it and it snowballed. Let's do this version since it's easy
for now.
While here I changed things so that `generate_deployment_refs()` now just uses
`_set_ref_immediate()` rather than requring a txn.
Also, AFAICS there was no test coverage of `generate_deployment_refs()`; I tried
commenting it out and at least `admin-test.sh` passed. Add some coverage of this
- I verified that with this commenting out bits of that function cause the test
to fail.
Closes: #1132
Approved by: jlebon
I resisted trying to do anything invasive here like fd-relative porting as our
coverage is weak. But this was all straightforward porting to decl-after-stmt
style.
Closes: #1153
Approved by: jlebon
Before commit e0346c1, a non-system repo could specify
remotes-config-dir and have remotes read from there. However, adding
remotes would only be done in the config dir for a system repo. Restore
that by respecting remotes-config-dir when no sysroot is found and
adding back the ostree_repo_is_system() check when adding remotes.
Closes: #1133Closes: #1151
Approved by: cgwalters
Include non-default deployments in the uEnv.txt file imported by
U-Boot. All the configurations beside the defaults will have
numerical suffix E.G. "kernel_image2" or "bootargs2".
Those U-Boot environment variables may be used from interactive boot
prompt or from "altbootcmd" script.
Closes: #1138
Approved by: cgwalters
Split the code that merge the system uEnv to new function. While we're here,
clean up the logic to e.g. use `ot_openat_ignore_enoent()`.
Closes: #1138
Approved by: cgwalters
There were some important ones there like a random `syncfs()`. The remaining
users are mostly blocked on the "fstatat enoent" case, I'll wait to port those.
Closes: #1150
Approved by: jlebon
Add keys from the signing homedir to the GpgVerifier used to look
for duplicate signatures. This will allow signatures from subkeys
to be canonicalised and recognised as already signed despite the
differing key ID, avoiding duplicate signatures.
Closes: https://github.com/ostreedev/ostree/issues/608Closes: #1092
Approved by: cgwalters
In the case the signature time was bad, a line prefix was missing from the
result of `ostree_gpg_verify_result_describe_variant()`.
Closes: #1092
Approved by: cgwalters
Revert the switch of _FINGERPRINT to giving the primary key ID
rather than the signing key ID, and instead add the primary
key ID as a new attribute which is available if the key is not
missing.
Closes: https://github.com/ostreedev/ostree/issues/608Closes: #1092
Approved by: cgwalters
Colin Walters
Jonathan Lebon
Philip Withnall
Matthew Leeds
Anton Gerasimov
Dan Nicholson
Guy Shapiro
Robert McQueen