I noticed this while debugging why I was seeing "2 metadata objects" fetched for
a different PR. I knew 1 was detached meta, but the other turned out to be this.
There's no reason to request a delta if the ref is unchanged.
Closes: #1220
Approved by: jlebon
Propagate the refspec_name from the OstreeRemote returned by an
OstreeRepoFinder through to the set_ref() call.
This changes ostree_repo_pull_with_options() to accept the
previously-disallowed combination of passing override-remote-name in
options and also setting a remote name in remote_name_or_baseurl.
ostree_repo_pull_with_options() will continue to pull using the remote
config named in remote_name_or_baseurl as before; but will now use the
remote name from override-remote-name when it’s setting the refs at the
end of the pull. This is consistent with the documentation for
override-remote-name.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1202
Approved by: cgwalters
Instead of returning just the keyring filename, return the entire
OstreeRemote, which has the keyring filename as one of its members. This
will simplify some upcoming changes, and allows slightly improved debug
logging.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1202
Approved by: cgwalters
If override-remote-name is specified in the options to
ostree_repo_pull_with_options(), but the remote_name_or_baseurl argument
is also set to a remote name, the override-remote-name would be leaked.
Note that this is currently an invalid configuration, so this leak is
basically never hit.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1202
Approved by: cgwalters
Followup for recent work in commits:
- 8a7a359709
- 1a9a473580
Keep track of how many objects we imported, and print that for `ostree
pull-local` (also do this even if noninteractive, like we did for `pull`).
In implementing this at first I used separate variables for import
from repo vs import from localcache, but that broke some of the
tests that checked those values.
It's easier to just merge them; we know from looking at whether or not
`remote_repo_local` is set whether or not we were doing a "HTTP pull with
localcache" versus a true `pull-local` and can use that when rendering status.
Closes: #1219
Approved by: jlebon
This is more efficient in the non-collection case; in the collection
case, the implementation of ostree_repo_resolve_collection_ref() needs
to be rewritten to improve efficiency.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1182
Approved by: cgwalters
Previously, collection–refs could only be pulled from a repository if it
had a summary file (which listed them). There was no way to pull from a
local repository which doesn’t have a summary file, and where the refs
were stored as refs/remotes/$remote/$ref, with a config section linking
that $remote to the queried collection ID.
Fix that by explicitly supporting pull_data->remote_repo_local in
fetch_ref_contents().
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1182
Approved by: cgwalters
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
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
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
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
A lof of the functions here are async and have nontrivial exits, but these ones
are all sync were straightforward ports.
Not prep for anything, just chipping away at porting.
Closes: #1146
Approved by: jlebon
We have `ot_ensure_unlinked_at()` for the "ignore ENOENT" case, and
`glnx_unlinkat()` otherwise. Port all in-tree callers to one or the other as
appropriate.
Just noticed an unprefixed error in the refs case and decided to do a tree-wide
check.
Closes: #1142
Approved by: jlebon
In almost all places. There are just a few exceptions; one tricky bit for
example is that the repo config must still have `mode=archive-z2`, since
`archive` used to mean something else. (We could very likely just get rid of
that check, but eh, later).
I also added a test that one can still do `ostree repo init --mode=archive-z2`.
Closes: #1125
Approved by: jlebon
For both flatpak and ostree-as-host, we really want to verify up front during
pulls that we're not being downgraded. Currently both flatpak and
`OstreeSysrootUpgrader` do this before deployments, but at that point we've
already downloaded all the data, which is annoying.
Closes: https://github.com/ostreedev/ostree/issues/687Closes: #1055
Approved by: jlebon
If one of the localcache repos has the exact same commit we resolved
from the remote, then we need to make sure to mark it as partial so that
we download the full tree.
Closes: #1074Closes: #1076
Approved by: cgwalters
Add a new error domain for GPG signing/verification errors, and use it
throughout libostree for describing verification errors. This replaces
various uses of G_IO_ERROR_FAILED, and one instance of
G_IO_ERROR_NOT_FOUND (for which some code in ot-builtin-show.c had to be
changed to ensure it was still handled correctly).
The use of a separate error domain allows failures in GPG operations to
be handled separately from network failures (where the summary file
could not be found to be downloaded, for example) or timeouts.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1064Closes: #1071
Approved by: mbarnes
When returning results from finding repos, set gpg-verify-summary=false
in their configs, since any pulls from such remotes will necessarily
involve collection IDs, and hence should be using the unsigned summary
support. In the intended deployment mode for P2P transmission of OSTree
refs, summaries *cannot* be signed, so setting gpg-verify-summary=true
would cause all the pulls to fail.
The unsigned summary support is the move of repository metadata from
the summary file (not spliceable) to the well-known ostree-metadata ref
(spliceable, as it can exist for multiple collection IDs in the same
repository).
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1066
Approved by: cgwalters
The API for downloading a summary file can legitimately return NULL for
the summary file contents when it returns TRUE (success). This indicates
an error 404 — the summary file was not found.
Two call sites were not handling that correctly, which was causing later
assertion failures.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1061Closes: #1065
Approved by: cgwalters
This catches a few failure modes in the pull code a little earlier,
before the incorrectly-NULL repo makes its way into a closure and a
worker thread, where the cause of the problem is harder to track down.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1058
Approved by: cgwalters
The intended behaviour of ostree_repo_find_remotes() is to return
results which have the latest version of at least one of the requested
refs. Results which have some of the requested refs, but don’t have the
latest version of any of them, should be ignored. The logic to do this
was broken in the case that a result contained a positive number of the
requested refs, but none of them were the latest version. (It previously
worked when the result contained none of the requested refs.)
Fix the counting to work correctly in both cases.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1058
Approved by: cgwalters
Part of cleaning up our usage of libglnx; we want to use what's in GLib where we
can.
Had to change a few .c files to `#include ostree.h` early on to pick up
autoptrs for the core types.
Closes: #1040
Approved by: jlebon
This is a continuation of addition of journaling to libostree; see
e.g. <https://github.com/ostreedev/ostree/pull/708>.
I wanted more information at the end of fetches; in particular
some details about the delta execution (what opcodes etc.), but
this is a first step: we log things like the transferred data
as well as whether or not GPG was enabled, etc.
One awkward thing about this is how we map the fetcher options like
`tls-ca-path` back out into an enum for the code to log. But eh, hard to fix
without a bigger refactoring.
Closes: #1032
Approved by: jlebon
These were previously private, but since we expect people to use them, let's add
`#define`s like we did for some of the other commit metadata.
Closes: #1028
Approved by: jlebon
Mostly for the latest `-Wmaybe-uninitialized` fix, but while here also port some
places to newer APIs.
Update submodule: libglnx
Closes: #1027
Approved by: jlebon
See: https://github.com/projectatomic/rpm-ostree/issues/885
If we get a successful Apache directory listing HTML when fetching what we
intend to be a ref, we'd dump the HTML into the error.
I did some scanning of the pull code, and this was the only case
I saw offhand where we were dumping text out into an error. Which
makes sense, since most of our formats are binary, the exeptions I
think are just `repo/config` and `repo/refs/`.
Closes: #1015
Approved by: mbarnes
Using the error prefixing in the delta processing allows us to
do new code style. Also strip trailing whitespace.
Use error prefixing in a few other random places. I didn't
hunt for all of them, just testing out the new API.
Use `glnx_fchmod()`. Also note I dropped one `fchmod (tmpf, 0600)`
which is no longer necessary.
Update submodule: libglnx
Closes: #1011
Approved by: jlebon
Prep for `ostree_repo_new_at()`. Down the line perhaps
we should extend libcurl to accept a file descriptor for cookies,
but this works OK for now.
Closes: #1010
Approved by: jlebon
This came up in <https://github.com/ostreedev/ostree/pull/982>; when
we added more direct local importing, we did it synchronously.
This was actually quite a regression when doing local pulls between different
modes; in particular between a bare mode and `archive`, as we were suddenly
doing gzip {de,}compression in the main thread.
Down the line actually...a simpler fix is probably to change things so that the
local path is really only used when we know we can hardlink; everything else
would go though the fetcher codepath but with `file://`.
But this isn't a lot more code, and the speed/interactivity win is large.
Note we're only doing content async with this patch. We could do metadata as
well; we have the object already local. But the metadata code path is messier,
and metadata objects are smaller.
Another area where this comes up is that in e.g. Fedora releng, most operations
talk to a NetApp via NFS. So this has the classic network filesystem problem
that operations that are normally cheap like `stat()` can actually have
nontrivial latency. Doing as much as possible in threads is better there too.
Closes: #1006
Approved by: jlebon
Currently in Fedora we don't sign summaries, and every use of
`rpm-ostree` would emit to the journal an error when we failed
to fetch it.
Fix this by having `OSTREE_FETCHER_REQUEST_OPTIONAL_CONTENT` tell the fetcher
not to journal 404 errors. While fixing this, we had a mix of two booleans vs
the flags; fix things so we consistently use the flags in the fetcher and pull
code.
Closes: #1004
Approved by: mbarnes
This verifies the collection and ref bindings in the commit metadata
against the collection ID we have stored in the remote config and ref
we want to pull from. For the HEAD commits, we also check if the
checksum of the commit we just fetched agrees with the checksum we
really wanted to pull from the ref.
For commits with explicitly specified checksums and without specified
refs, we only verify if the commit has the bindings. We are able to
only verify the collection binding, though.
Closes: #972
Approved by: cgwalters
In the storage PR I was trying to do a `pull-local` of the whole
`/ostree/repo` on the system, which ended up triggering a `g_critical()`
in the collections code, since we tried to parse a remote-prefixed ref
`fedora:fedora/26/x86_64/atomic-host` as a ref.
I'm not sure offhand what our behavior in this case *should* be. I
think git only clones local refs, but I need to check.
This corner case arises only with `pull-local`. But in any case,
while we were previously saying this is programmer error, since it's
so easy to pass various unchecked input into the pull machinery,
make invalid refs an explicit error.
Closes: #992
Approved by: jlebon
This is a lot like `git clone --reference`, but we chose "localcache" as the
term "reference" is already used.
The main use case I'm targeting this for is the Fedora Atomic Host installer
case where we embed the repo content in the installer, but we may want to
kickstart and download newer content. There, while we want to get a newer ref,
we can still use the local repo as an object cache, since we have it sitting
there in memory anyways.
Another case is where one has a host ostree (say e.g. Fedora Atomic
Workstation), and one wants to create a local archive mirror of FAH. Then one
can use `pull --reference /ostree/repo` and pull the common objects (e.g.
contents of `bash.rpm` etc.)
Closes: https://github.com/ostreedev/ostree/issues/975Closes: #982
Approved by: jlebon
These are regression from #971. We were stuffing a pointer size inside a
variable of integer size. So the assignment was spilling over into other
variables' storage space. Actually use a gpointer and GPOINTER_TO_[U]INT
as was done originally.
Also bump libglnx which has static checks for this error in the future.
Update submodule: libglnx
Closes: #990
Approved by: cgwalters
Obviously very minor, but I noticed this while working on `pull --reference`. If
we have a local repo, we'll have already done a hardlink and copied the detached
metadata too, so there's no reason to request it again via the fetcher path.
Closes: #978
Approved by: jlebon
I'd mostly been avoiding this file since there's always patches outstanding, but
these few functions shouldn't conflict much.
Closes: #979
Approved by: jlebon
Use the new macros introduced recently in libglnx to make iterating over
hash tables cleaner. This is just a start, it does not migrate the whole
tree.
Update submodule: libglnx
Closes: #971
Approved by: cgwalters
There's lots of mechanically replacing `OtTmpFile` with `GLnxTmpfile`;
the biggest changes are in the commit path. Symlink commits are now
very clearly separated from regular files. Symlinks are `OtCleanupUnlinkat`,
and regular files are `GLnxTmpfile`.
The commit codepath separates those as `_ostree_repo_commit_path_final()` and
`_ostree_repo_commit_tmpf_final()`. A nice aspect of all of this is that they
both *consume* the temporary on success. This avoids an extra spurious
`unlink()` call.
One of the biggest bits of code motion is in `commit_loose_regfile_object()`,
which no longer needs to care about symlinks. For the most parth though it's
just removing conditionals.
Update submodule: libglnx
Closes: #958
Approved by: jlebon
Rather than duplicating the code. This introduces no functional changes.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #961
Approved by: cgwalters
Check that it’s in normal form and has the correct type when loading it,
since it could come from an untrusted source.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #961
Approved by: cgwalters
This makes no difference to the validity of the code, since any summary
file loaded from the cache will be verified before being read anyway;
but it will make some upcoming changes a little simpler.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #961
Approved by: cgwalters
Colin Walters
Philip Withnall
Matthew Leeds
Jonathan Lebon
Krzesimir Nowak