ostree.git
8 years agolib/pull: When --require-static-deltas, use them even for file:/// repos
Colin Walters [Fri, 27 Oct 2017 21:24:49 +0000 (17:24 -0400)]
lib/pull: When --require-static-deltas, use them even for file:/// repos

I didn't fully spelunk this, but from what `static-delta-generate-crosscheck.sh`
had, we appeared to be doing this before, and it's clearly useful for local
testing rather than needing to spin up a HTTP server.

Closes: #1313
Approved by: jlebon

8 years agolib: Minor static delta fixes
Colin Walters [Fri, 27 Oct 2017 20:14:02 +0000 (16:14 -0400)]
lib: Minor static delta fixes

First, the manual crosscheck script bitrotted; it got caught up
in the "use libtest repo creation wrapper" bit, and also it
seems like at some point `pull --require-static-deltas` changed
meaning when dealing with `file:///` repos.  I have more work to
unwind that.

Next, I'm seeing a delta failure which looks like a static delta
miscompilation with rollsums; change the compiler to print out
the source object too, which helped me debug this.

And finally in the processing code, fix incorrect error prefixing, which was
misleading.

Closes: #1311
Approved by: ashcrow

8 years agolib/deltas: Fix change to use pread() in write opcode
Colin Walters [Fri, 27 Oct 2017 20:57:10 +0000 (16:57 -0400)]
lib/deltas: Fix change to use pread() in write opcode

Fixes: 93457071cb5d47c08b60d3244f9632725634010a "lib/deltas: Use pread() instead of lseek()+read()"
Caught this when trying to test alex's patch locally. I am going to review our
static delta pulls and try to get something more comprehensive locally. But in
the meantime this patch is clearly right.

Closes: #1312
Approved by: jlebon

8 years agostatic deltas: Process each part as soon as its done
Alexander Larsson [Wed, 25 Oct 2017 20:32:02 +0000 (22:32 +0200)]
static deltas: Process each part as soon as its done

Directly when we allocate a new part we finish the old one,
writing the compressed data to a temporary file and generating
the delta header for it.

When all these are done we loop over them and collect the headers,
sizes and either copy the tempfile data into the inlined superblock
or link the tempfiles to disk with the proper names.

Closes: #1309
Approved by: cgwalters

8 years agostatic-delta-compilation: Move some things around
Alexander Larsson [Wed, 25 Oct 2017 19:53:38 +0000 (21:53 +0200)]
static-delta-compilation: Move some things around

We will do some changes later that need these earliers, so move them up.

Closes: #1309
Approved by: cgwalters

8 years agostatic deltas: Use OtVariantBuilder to create deltas
Alexander Larsson [Wed, 25 Oct 2017 18:45:21 +0000 (20:45 +0200)]
static deltas: Use OtVariantBuilder to create deltas

This allows us to create the final delta desciptor directly on disk
rather than having it all in memory. This is nice because it can
become quite large if inlined parts are used.

Note however, that we currently generate all the delta parts in
memory before adding them to the delta, so we still keep all individual
parts in memory. Fixing that is the next step.

Closes: #1309
Approved by: cgwalters

8 years agoAdd OtVariantBuilder
Alexander Larsson [Wed, 25 Oct 2017 17:31:48 +0000 (19:31 +0200)]
Add OtVariantBuilder

This is similar to GVariantBuilder in that it constructs variant
containers, but it writes it directly to a file descriptor rather
than keep the entier thing in memory. This is useful to create large
variants without using a lot of memory.

Closes: #1309
Approved by: cgwalters

8 years agoostree-system-generator: Include <libglnx.h> for autocleanups
Simon McVittie [Fri, 27 Oct 2017 13:41:31 +0000 (14:41 +0100)]
ostree-system-generator: Include <libglnx.h> for autocleanups

g_autoptr was new in GLib 2.44, but we officially only require 2.40,
so we need to use the backport in libglnx.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Closes: #1310
Approved by: cgwalters

8 years agolib/repo: Fix loading commitstate with parent repos
Colin Walters [Wed, 25 Oct 2017 17:13:17 +0000 (13:13 -0400)]
lib/repo: Fix loading commitstate with parent repos

This makes the code nicer too. Properly unit testing this though really wants
like a whole set of stuff around parent repos...but we do have coverage of the
non-parent path in the current pull tests.

Closes: https://github.com/ostreedev/ostree/issues/1306
Closes: #1308
Approved by: alexlarsson

8 years agolib/repo-finder-mount: Ignore mounts which have a ‘system’ file system
Philip Withnall [Wed, 25 Oct 2017 14:49:49 +0000 (15:49 +0100)]
lib/repo-finder-mount: Ignore mounts which have a ‘system’ file system

For example, tmpfs or a cgroup file system. This is basically an
optimisation of the list of file systems we check for repositories,
since we would never expect any of these file systems to be capable of
containing a repository.

Depends on the new API from
https://bugzilla.gnome.org/show_bug.cgi?id=788927.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1307
Approved by: cgwalters

8 years agotree-wide: Use autoptr for OstreeKernelArgs
Colin Walters [Tue, 24 Oct 2017 14:10:09 +0000 (10:10 -0400)]
tree-wide: Use autoptr for OstreeKernelArgs

Much nicer looking.  Prep for more cleanup from
https://github.com/projectatomic/rpm-ostree/pull/1013

Closes: #1302
Approved by: peterbaouoft

8 years agolib/repo: Properly handle NULL homedir when signing commit
Dan Nicholson [Tue, 24 Oct 2017 19:27:17 +0000 (19:27 +0000)]
lib/repo: Properly handle NULL homedir when signing commit

Without this, ostree_repo_sign_commit throws a critical message when no
homedir is provided:

(ostree gpg-sign:5034): GLib-GIO-CRITICAL **: g_file_new_for_path: assertion 'path != NULL' failed

Closes: #1305
Approved by: cgwalters

8 years agolib/repo-finder-mount: Fix propagating NULL errors
Philip Withnall [Tue, 24 Oct 2017 14:26:49 +0000 (15:26 +0100)]
lib/repo-finder-mount: Fix propagating NULL errors

These two code paths tried to propagate errors which had never been set.
Set new errors instead.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1303
Approved by: cgwalters

8 years agoci: Update centos container to current again
Colin Walters [Tue, 24 Oct 2017 14:27:23 +0000 (10:27 -0400)]
ci: Update centos container to current again

Repo sync issue should be cleared now, and for some reason the previous
container is already GC'd.

Closes: #1304
Approved by: peterbaouoft

8 years agolib/repo-finder-avahi: Fix memory corruption of a GVariantIter
Philip Withnall [Mon, 23 Oct 2017 15:32:49 +0000 (16:32 +0100)]
lib/repo-finder-avahi: Fix memory corruption of a GVariantIter

A GVariantIter* was being passed to a GVariant format string varargs,
rather than a GVariantIter**. This resulted in memory corruption.

So we can continue to reuse ref_map throughout the function, make it a
GVariantIter* rather than a stack-allocated GVariantIter.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1301
Approved by: cgwalters

8 years agolib/commit: Fix hardlink checkout commit with bare-user + mod xattrs
Colin Walters [Fri, 20 Oct 2017 18:15:14 +0000 (14:15 -0400)]
lib/commit: Fix hardlink checkout commit with bare-user + mod xattrs

This is more subtle fallout from:
https://github.com/ostreedev/ostree/pull/1170
AKA commit: 8fe45362578a43260876134d6547ebd0bb2485c3

Before, if we found a devino cache hit, we'd use it unconditionally.

Recall that `bare-user` repositories are very special in that they're the only
mode where the on disk state ("physical state") is not the "real" state. The
latter is stored in the `user.ostreemeta` xattr. (`bare-user` repos are also
highly special in that symlinks are regular files physically, but that's not
immediately relevant here).

Since we now have `bare-user-only` for the "pure unprivileged container" case,
`bare-user` should just be used for "OS builds" which have nonzero uids (and
possibly SELinux labels etc.)

In an experimental tool I'm writing "skopeo2ostree" which imports OCI images
into refs, then squashes them together into a single final commit, we lost the
the `81` group ID for `/usr/libexec/dbus-1/dbus-daemon-launch-helper`.

This happened because the commit code was loading the "physical" disk state,
where the uid/gid are zero because that's the uid I happened to be using. We
didn't just directly do the link speedup because I was using `--selinux-policy`
which caused the xattrs to change, which caused us to re-commit objects from the
physical state.

The unit test I added actually doesn't quite trigger this, but I left
it because "why not".  Really testing this requires the installed test
which uses SELinux policy from `/`.

The behavior without this fix looks like:

```
-00755 0 0     12 { [(b'user.ostreemeta', [byte 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x51, 0x00, 0x00, 0x81, 0xed]), (b'security.selinux', b'system_u:object_r:lib_t:s0')] } /usr/lib/dbus-daemon-helper
```

which was obviously totally broken - we shouldn't be picking up the
`user.ostreemeta` xattr and actually committing it of course.

Closes: #1297
Approved by: jlebon

8 years agolib/sysroot: Expand docs for ostree_sysroot_get_repo()
Philip Withnall [Mon, 23 Oct 2017 14:59:31 +0000 (15:59 +0100)]
lib/sysroot: Expand docs for ostree_sysroot_get_repo()

Add some missing annotations and clarify that it always returns an open
repository on success.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1300
Approved by: cgwalters

8 years agocore: Add standard SOURCE_TITLE metadata key
Colin Walters [Thu, 19 Oct 2017 18:28:02 +0000 (14:28 -0400)]
core: Add standard SOURCE_TITLE metadata key

This is a freeform string useful to track/display when a commit is "derived"
from some other format.  For example, in the rpm-ostree test we make a
`vmcheck` ref that conceptually overlays the default ref like
`fedora-atomic:fedora/26/x86_64/atomic-host`.

My current patch sets the source title to e.g.
"Dev overlay on fedora-atomic:fedora/26/x86_64/atomic-host".

Another case I'm working on now is importing OCI images to use
as host images.  For that case, the source title is
With this patch we could then set the original OCI image name + tag
as the source name, like:
"oci:cgwalters/demo-custom-fedora-atomic-host:26".

Closes: #1296
Approved by: jlebon

8 years agobin/status: Inline metadata parsing for versions
Colin Walters [Fri, 20 Oct 2017 14:19:00 +0000 (10:19 -0400)]
bin/status: Inline metadata parsing for versions

Pull out the commit metadata explicitly; still just rendering the version, but
this is prep for rendering other metadata keys.

Closes: #1296
Approved by: jlebon

8 years agobin/status: Port to new style
Colin Walters [Fri, 20 Oct 2017 13:19:52 +0000 (09:19 -0400)]
bin/status: Port to new style

No functional changes; prep for more work.

Closes: #1296
Approved by: jlebon

8 years agodocs/related-projects: Add Google Usenix link
Colin Walters [Sat, 21 Oct 2017 14:10:34 +0000 (10:10 -0400)]
docs/related-projects: Add Google Usenix link

Closes: https://github.com/ostreedev/ostree/issues/1284
Closes: #1299
Approved by: cgwalters

8 years agorepo: Add wrapper function for setting devino cache on checkout opts
Colin Walters [Thu, 19 Oct 2017 14:25:59 +0000 (10:25 -0400)]
repo: Add wrapper function for setting devino cache on checkout opts

I was trying to use this with pygobject for an OCI+ostree project, and pygobject
rejected simply assigning to the field (understandably, since it can't bind the
lifetime together).

Add a wrapper function, which is still unsafe, but hides that unsafety
where most people shouldn't find it.  And if they do...well, sorry,
Rust wasn't invented when ostree was started.

Closes: #1295
Approved by: pwithnall

8 years agolib/checkout: For "process whiteouts" mode, replace directories too
Colin Walters [Thu, 19 Oct 2017 15:18:00 +0000 (11:18 -0400)]
lib/checkout: For "process whiteouts" mode, replace directories too

I'm playing around with some ostree ⇔ OCI/Docker bits, and ran
into this while importing an OCI image that built from the Fedora
base image where `/home` is a regular directory, and I added a layer
that did the ostree bits of moving it to `/var` and leaving a symlink.

OCI/Docker supports this.  Now since "process whiteouts" is really the
"enable OCI/Docker" mode, let's only replace dirs if that's enabled.
This leaves the `UNION_FILES` targeted for its original use case
which is unioning components/packages.  (Although that use case itself
is now a bit superceded by `UNION_IDENTICAL`, but eh).

Closes: #1294
Approved by: jlebon

8 years agoostree: provide command description in a better place
Ruixin Bao [Tue, 17 Oct 2017 20:42:04 +0000 (20:42 +0000)]
ostree: provide command description in a better place

This is similar idea as
https://github.com/projectatomic/rpm-ostree/pull/916/commits/5c0bf889155573feec108501ce849ac09a0c9567,

The duplicated description is now removed, and the description
of the command is now displayed beneath the Usage.

For example:
ostree cat -h will output the following:

"Usage:
  ostree cat [OPTION?] COMMIT PATH...

Concatenate contents of files"

Closes: #1267
Approved by: cgwalters

8 years agoostree: move flags into command struct, pass down through builtins
Ruixin Bao [Tue, 17 Oct 2017 17:25:04 +0000 (17:25 +0000)]
ostree: move flags into command struct, pass down through builtins

This is a similar approach as
https://github.com/projectatomic/rpm-ostree/commit/12c34bb2491a07079c911ef26401fee939e5573c.

One thing to note is when we parse the admin related functions,
we still keep the old admin related flags, and added a new parameter
to represent the command struct.

This allows us to identify the caller of the function, making it
easier for us to possibly deduplicate the subcommand handling in
the future. A similar approach is done in rpm-ostree:
https://github.com/projectatomic/rpm-ostree/commit/83aeb018c1012c7a43783c09b74ec71bc9c45826

This also makes it easier for us to change the prototype of the function.
If we want to add something new in the future, we won't need to touch every prototype.

Closes: #1267
Approved by: cgwalters

8 years agoostree: Describe subcommands in help output
Ruixin Bao [Fri, 13 Oct 2017 16:15:13 +0000 (16:15 +0000)]
ostree: Describe subcommands in help output

Added a description argument to all type
of commands. Now when we include -h or --help
for commands that contain subcommands, the description
for those subcommands are shown.

The added subcommands help will be provided to the following commands:
- ostree -h
- ostree admin -h
- ostree admin instutil -h
- ostree remote -h
- ostree static-delta -h

Closes: #1267
Approved by: cgwalters

8 years agolib/repo-finder: Add OstreeRepoFinderOverride
Philip Withnall [Mon, 16 Oct 2017 15:18:03 +0000 (16:18 +0100)]
lib/repo-finder: Add OstreeRepoFinderOverride

This is another OstreeRepoFinder implementation; it returns results from
a given set of URIs. It’s designed to be used for implementing user
overrides to other repo-finders, or for implementing unit tests.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1281
Approved by: mwleeds

8 years agolib/repo-finder-avahi: Fix a leak in a GVariantIter loop
Philip Withnall [Thu, 19 Oct 2017 13:04:10 +0000 (14:04 +0100)]
lib/repo-finder-avahi: Fix a leak in a GVariantIter loop

Use g_variant_iter_loop() rather than next(), since it automatically
handles freeing the child memory each iteration. Previously, we leaked
it for all but the last iteration.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1293
Approved by: cgwalters

8 years agoDisallow refs starting with a non-letter or digit
Colin Walters [Wed, 18 Oct 2017 00:53:27 +0000 (20:53 -0400)]
Disallow refs starting with a non-letter or digit

Change the regexp for validating refs to require at least one letter or digit
before allowing the other special chars in the set `[.-_]`. Names that start
with `.` are traditionally Unix hidden files; let's ignore them under the
assumption they're metadata for some other tool, and we don't want to
potentially conflict with the special `.` and `..` Unix directory entries.
Further, names starting with `-` are problematic for Unix cmdline option
processing; there's no good reason to support that. Finally, disallow `_` just
on general principle - it's simpler to say that ref identifiers must start with
a letter or digit.

We also ignore any existing files (that might be previously created refs) that
start with `.` in the `refs/` directory - there's a Red Hat tool for content
management that injects `.rsync` files, which is why this patch was first
written.

V1: Update to ban all refs starting with a non-letter/digit, and
    also add another call to `ostree_validate_rev` in the pull
    code.

Closes: https://github.com/ostreedev/ostree/issues/1285
Closes: #1286
Approved by: jlebon

8 years agobuild: Do *.am.inc substitutions from make correctly
Dan Nicholson [Wed, 18 Oct 2017 16:17:19 +0000 (16:17 +0000)]
build: Do *.am.inc substitutions from make correctly

The string needs to be escaped with a $ since we want to replace the
literal string $(path). Without this make will run the command with the
value of the variable, which won't match anything in the input Makefile
stub.

Closes: #1291
Approved by: smcv

8 years agolib/commit: fix checking flag with bitwise OR
Jonathan Lebon [Wed, 18 Oct 2017 13:33:58 +0000 (13:33 +0000)]
lib/commit: fix checking flag with bitwise OR

Caught by Coverity.

Coverity CID: 1458339

Closes: #1290
Approved by: cgwalters

8 years agolib/commit: (refactor) Clean up delta bare write API
Colin Walters [Fri, 13 Oct 2017 21:40:16 +0000 (17:40 -0400)]
lib/commit: (refactor) Clean up delta bare write API

The way `_ostree_repo_open_content_bare()` did both looking for the object and
possibly creating a new fd was just weird and inconsistent with e.g. the pull
code where we always call `has_object()` first.

Just call `has_object()` in the delta paths that used this too, making the
implementation right now a thin wrapper around
`glnx_open_tmpfile_linkable_at()`, but this is prep for a later patch which does
more.

Closes: #1283
Approved by: jlebon

8 years agolib/repo: Update summary code to use newer hashing API
Colin Walters [Wed, 18 Oct 2017 01:13:20 +0000 (21:13 -0400)]
lib/repo: Update summary code to use newer hashing API

And drop the unnecessary wrapper.

Closes: #1287
Approved by: jlebon

8 years agolib/pull: Update summary checksum code to use new checksum API
Colin Walters [Wed, 18 Oct 2017 01:07:19 +0000 (21:07 -0400)]
lib/pull: Update summary checksum code to use new checksum API

This is another case where making an input stream out of a memory buffer is a
bit silly; just hash the `GBytes` directly.

Closes: #1287
Approved by: jlebon

8 years agodocs: mention the $OSTREE_REPO environment variable
Marcus Folkesson [Tue, 17 Oct 2017 19:03:23 +0000 (21:03 +0200)]
docs: mention the $OSTREE_REPO environment variable

$OSTREE_REPO may be set to override the default location
of the repository.

Link: https://mail.gnome.org/archives/ostree-list/2017-October/msg00003.html
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
Closes: #1282
Approved by: cgwalters

8 years agosyntax-check: Add a rule to enforce glnx_autofd over glnx_fd_close
Colin Walters [Tue, 17 Oct 2017 15:18:26 +0000 (11:18 -0400)]
syntax-check: Add a rule to enforce glnx_autofd over glnx_fd_close

And fix the one final use.

Closes: #1280
Approved by: jlebon

8 years agolib/commit: Use direct fd xattr operations again on regular files
Colin Walters [Mon, 16 Oct 2017 20:08:26 +0000 (16:08 -0400)]
lib/commit: Use direct fd xattr operations again on regular files

A side effect of commit 8fe45362578a43260876134d6547ebd0bb2485c3 is that
we started listing all xattrs even for files with device/inode matches;
further, we did that using the dfd/name which means we went through
the `/proc` path, which is slower and uglier.

Noticed this in strace while looking at adoption code.

Closes: #1280
Approved by: jlebon

8 years agolib/commit: Fix indentation in file commit code
Colin Walters [Mon, 16 Oct 2017 20:07:30 +0000 (16:07 -0400)]
lib/commit: Fix indentation in file commit code

No functional changes; the indentation was off here and it was
confusing me working on another patch.

Closes: #1280
Approved by: jlebon

8 years agolib/commit: Avoid trying to delete `.` with _CONSUME flag
Colin Walters [Mon, 16 Oct 2017 19:04:48 +0000 (15:04 -0400)]
lib/commit: Avoid trying to delete `.` with _CONSUME flag

This helps port rpm-ostree.

Closes: #1278
Approved by: jlebon

8 years agolib/deltas: Use pread() instead of lseek()+read()
Colin Walters [Sat, 14 Oct 2017 00:51:39 +0000 (20:51 -0400)]
lib/deltas: Use pread() instead of lseek()+read()

That's why the syscall was invented, so let's use it. Just noticed while reading
the code while working on another patch.

Closes: #1270
Approved by: jlebon

8 years agolib/deltas: Use fastpath for regfile writes for bare-user-only
Colin Walters [Sat, 14 Oct 2017 00:50:08 +0000 (20:50 -0400)]
lib/deltas: Use fastpath for regfile writes for bare-user-only

Noticed this omission while looking at the code while working on another patch.

Closes: #1270
Approved by: jlebon

8 years agotests: Add test-pull-bareuseronly
Colin Walters [Fri, 13 Oct 2017 22:14:17 +0000 (18:14 -0400)]
tests: Add test-pull-bareuseronly

I was going to fix a bug in the static deltas code and I noticed
we were missing `pull-test.sh` coverage for bareuseronly 🙈.

Obviously fixing this requires duplicating some of the bits we have in
`basic-test.sh`; need to hoist that into `libtest.sh`. For now though let's get
the coverage.

Closes: #1270
Approved by: jlebon

8 years agolib/sysroot: Fix error handling when mounting overlayfs fails
Colin Walters [Mon, 16 Oct 2017 19:29:38 +0000 (15:29 -0400)]
lib/sysroot: Fix error handling when mounting overlayfs fails

This isn't perfect, but at least we fix an error-overwrite error, and in
practice `ostree admin unlock` isn't wrapped by `rpm-ostree` yet, so spew to
stderr is OK.

Closes: https://github.com/ostreedev/ostree/issues/1273
Closes: #1279
Approved by: guyshapiro

8 years agolib: Use a common helper function to compare checksums
Colin Walters [Mon, 16 Oct 2017 18:35:17 +0000 (14:35 -0400)]
lib: Use a common helper function to compare checksums

So we get a consistent error message; came up in a PR review.

Closes: #1277
Approved by: jlebon

8 years agolib/commit: Implement "adoption" with CONSUME flag
Colin Walters [Sat, 14 Oct 2017 02:17:56 +0000 (22:17 -0400)]
lib/commit: Implement "adoption" with CONSUME flag

For checkouts that are on the same device, for regular files we can simply
"adopt" existing files. This is useful in the "build from subtrees" pattern that
happens with e.g. `rpm-ostree install` as well as flatpak and gnome-continuous.

New files are things like an updated `ldconfig` cache, etc. And particularly for
`rpm-ostree` we always regenerate the rpmdb, which for e.g. this workstation is
`61MB`.

We probably should have done this from the start, and instead had a `--copy`
flag to commit, but obviously we have to be backwards compatible.

There's more to do here - the biggest gap is probably for `bare-user` repos,
which are often used with things like `rpm-ostree compose tree` for host
systems. But we can do that later.

Closes: #1272
Approved by: jlebon

8 years agotests/basic: Add missing ${COMMIT_ARGS} for bare-user-only
Colin Walters [Sat, 14 Oct 2017 14:17:02 +0000 (10:17 -0400)]
tests/basic: Add missing ${COMMIT_ARGS} for bare-user-only

I was working on "adopt" and hit corruption; turns out we were missing the
`--canonical-permissions` arg in this existing test.

(Need to abstract all of this more)

Closes: #1272
Approved by: jlebon

8 years agobuild: Fix bsdiff Makefile circular dependency
Dan Nicholson [Mon, 16 Oct 2017 16:24:16 +0000 (16:24 +0000)]
build: Fix bsdiff Makefile circular dependency

The intended use was to have the .am.inc generated from the .am like the
libglnx one. Without this, make was detecting a circular dependency and
dropping the rule:

  make: Circular bsdiff/Makefile-bsdiff.am.inc <- bsdiff/Makefile-bsdiff.am.inc dependency dropped.

Closes: #1276
Approved by: jlebon

8 years agolib/pull: Also do commit GPG verification before writing
Colin Walters [Fri, 13 Oct 2017 18:51:34 +0000 (14:51 -0400)]
lib/pull: Also do commit GPG verification before writing

I was working on a patch to do build on the work done to
import content objects async to do the same for metadata, but right
now we basically rely on writing them first to do the GPG verification
when scanning.

Things will be cleaner for that if we can pass the commit object directly into
`scan_commit_object()` and consistently use `gpg_verify_unwritten_commit()`.

We're careful here to continue to do it both ways (but at most one time), to
account for the case where a bad commit has been pulled and written - we need to
keep failing GPG verification there.

Closes: #1269
Approved by: jlebon

8 years agolib/pull: Drop duplicate/different error for GPG but no detached meta
Colin Walters [Fri, 13 Oct 2017 19:22:10 +0000 (15:22 -0400)]
lib/pull: Drop duplicate/different error for GPG but no detached meta

Prep for a later patch to do GPG verification before writing commit objects;
`_ostree_repo_gpg_verify_with_metadata()` already handles this, and so dropping
this gives us consistent error messages.

Closes: #1269
Approved by: jlebon

8 years agoMake sure *.am.inc are up to date before `make dist`
Simon McVittie [Tue, 3 Oct 2017 20:26:50 +0000 (21:26 +0100)]
Make sure *.am.inc are up to date before `make dist`

v2017.12 didn't include test-libglnx-shutil.c, but if you re-run
autogen.sh (as we do in Debian, to update the Autotools build system)
it will try to build it.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Closes: #1274
Approved by: cgwalters

8 years agoCope with xattr syscalls raising EOPNOTSUPP
Simon McVittie [Mon, 16 Oct 2017 11:51:04 +0000 (12:51 +0100)]
Cope with xattr syscalls raising EOPNOTSUPP

ENOTSUP and EOPNOTSUPP are numerically equal on most Linux ports,
but inexplicably differ on PA-RISC (hppa) and possibly other
rare architectures.

Signed-off-by: Simon McVittie <smcv@collabora.com>
Closes: #1275
Approved by: cgwalters

8 years agolib/checkout: fallback to checksum for UNION_IDENTICAL
Jonathan Lebon [Tue, 10 Oct 2017 18:14:10 +0000 (18:14 +0000)]
lib/checkout: fallback to checksum for UNION_IDENTICAL

There's a subtle issue going on with the way we use `UNION_IDENTICAL`
now in rpm-ostree. Basically, the crux of the issue is that we checkout
the whole tree from the system repo, but then overlay packages by
checking out from the pkgcache repo. This is an easy way to break the
assumption that we will be merging hardlinks from the same repo.

This ends up causing issues like:
https://github.com/projectatomic/rpm-ostree/issues/1047

There, `vim-minimal` is already part of the host and has an object for
`/usr/share/man/man1/ex.1.gz`. `vim-common` has that same file, but
because it's unpacked in the pkgcache repo first, the hardlinks are not
the same.

There are a few ways we *could* work around this in rpm-ostree itself,
e.g. by re-establishing hardlinks when we do the content pull into the
system repo, but it still felt somewhat hacky. Let's just do this the
proper way and fall back to checksumming the target file if needed,
which is what librpm does as well in this case. Note that we only
checksum if they're not hard links, but they're the same size.

Closes: #1258
Approved by: cgwalters

8 years agolib/utils: Check for invalid UTF-8 in filenames
Matthew Leeds [Fri, 13 Oct 2017 21:49:07 +0000 (14:49 -0700)]
lib/utils: Check for invalid UTF-8 in filenames

In case a filename contains invalid UTF-8 characters, libostree will
pass it to g_variant_builder_add() in create_tree_variant_from_hashes()
anyway, which leads to a critical warning from glib and an invalid
commit. This commit makes ostree print a useful error and exit instead.

Closes: #1271
Approved by: cgwalters

8 years agolib/deltas: Check cancellable during processing
Colin Walters [Thu, 12 Oct 2017 14:46:25 +0000 (10:46 -0400)]
lib/deltas: Check cancellable during processing

Let's react to `Ctrl-C` faster here. Noticed while I was doing an update on my
desktop and playing with cancellation.

Closes: #1266
Approved by: jlebon

8 years agolib/core: add ostree_checksum_file_at API
Jonathan Lebon [Wed, 11 Oct 2017 14:52:02 +0000 (14:52 +0000)]
lib/core: add ostree_checksum_file_at API

This is like `ostree_checksum_file` but fd-relative. This will be used
by https://github.com/ostreedev/ostree/pull/1258.

AFAICT, we actually didn't have any tests that check the `checksum` CLI.
Add a basic one here to test the old code as well as the new code.

Closes: #1263
Approved by: cgwalters

8 years agoostree/checksum: port to new decl style
Jonathan Lebon [Wed, 11 Oct 2017 15:46:52 +0000 (15:46 +0000)]
ostree/checksum: port to new decl style

No functional changes, prep for patch. (Well, I did add a new `success`
member in the async struct so that we return `FALSE` if we failed).

Closes: #1263
Approved by: cgwalters

8 years agotests: drop unused variable
Jonathan Lebon [Tue, 10 Oct 2017 18:13:40 +0000 (18:13 +0000)]
tests: drop unused variable

Closes: #1263
Approved by: cgwalters

8 years agolib/deploy: Use _exit() for FIFREEZE watchdog
Colin Walters [Wed, 11 Oct 2017 21:02:02 +0000 (17:02 -0400)]
lib/deploy: Use _exit() for FIFREEZE watchdog

This works around an (IMO) SpiderMonkey bug - it tries to
clean up in a shared library destructor, but doesn't install a
`pthread_atfork()` handler to unset its state.

Closes: https://github.com/ostreedev/ostree/issues/1262
Closes: #1264
Approved by: dbnicholson

8 years agotree-wide: Update to new libglnx fd APIs
Colin Walters [Wed, 11 Oct 2017 13:41:31 +0000 (09:41 -0400)]
tree-wide: Update to new libglnx fd APIs

This ends up a lot better IMO.  This commit is *mostly* just
`s/glnx_close_fd/glnx_autofd`, but there's also a number of hunks like:

```
-  if (self->sysroot_fd != -1)
-    {
-      (void) close (self->sysroot_fd);
-      self->sysroot_fd = -1;
-    }
+  glnx_close_fd (&self->sysroot_fd);
```

Update submodule: libglnx

Closes: #1259
Approved by: jlebon

8 years agolib/commit: Make -path commit helper API private
Colin Walters [Mon, 9 Oct 2017 14:05:20 +0000 (10:05 -0400)]
lib/commit: Make -path commit helper API private

It's no longer called directly by the pull code, so make it static.

The goal here is to have the pull and local-fs commit paths use higher level
more efficient APIs, and eventually make those APIs public.

Closes: #1257
Approved by: jlebon

8 years agolib/core: Use GBytes for file headers
Colin Walters [Sun, 8 Oct 2017 19:55:35 +0000 (15:55 -0400)]
lib/core: Use GBytes for file headers

This simplifies a lot of code; the header function was structured
to write to an input stream, but many callers only wanted the checksum,
so it's simpler (and error-free) to simply allocate a whole buffer
and checksum that.

For the callers that want to write it, it's also still simpler to allocate the
buffer and write the whole thing rather than having this function do the
writing.

A lot of the complexity here again is a legacy of the packfile code, which is
dead.

This is prep for faster regfile commits where we can avoid `G{In,Out}putStream`.

Closes: #1257
Approved by: jlebon

8 years agolib/core: Port a few functions to decl-after-stmt
Colin Walters [Sat, 7 Oct 2017 16:11:05 +0000 (12:11 -0400)]
lib/core: Port a few functions to decl-after-stmt

No functional changes, just prep for more work.

Closes: #1257
Approved by: jlebon

8 years agolib/core: (refactor) Drop wrapper and unneeded args for variant writing
Colin Walters [Sat, 7 Oct 2017 13:36:51 +0000 (09:36 -0400)]
lib/core: (refactor) Drop wrapper and unneeded args for variant writing

Nothing was using the `bytes_written` data (we always discard partially written
tmpfiles), so simplify everything by dropping it. Further, we always passed an
offset of `0`, so drop that argument too. (I believe that this was previously
used by the "pack files" code that we deleted long ago)

Second, we had an unnecessary internal wrapper for this function; drop that too.

Closes: #1257
Approved by: jlebon

8 years agoci: compile f26/c7-primary with libcurl/openssl
Jonathan Lebon [Wed, 11 Oct 2017 14:54:59 +0000 (14:54 +0000)]
ci: compile f26/c7-primary with libcurl/openssl

This is how ostree is shipped in those distros. We already have
alternative testsuites for testing other HTTP & crypto backends.

Closes: #1261
Approved by: cgwalters

8 years agolibotutil/checksum-utils: fix openssl compilation
Jonathan Lebon [Wed, 11 Oct 2017 14:52:56 +0000 (14:52 +0000)]
libotutil/checksum-utils: fix openssl compilation

Closes: #1261
Approved by: cgwalters

8 years agolib/deploy: Ignore FIFREEZE/FITHAW errors when already in state
Dan Nicholson [Wed, 11 Oct 2017 14:03:10 +0000 (14:03 +0000)]
lib/deploy: Ignore FIFREEZE/FITHAW errors when already in state

If the filesystem is already frozen, FIFREEZE returns EBUSY, and if the
filesystem is already thawed, FITHAW returns EINVAL. It's very unlikely
these issues would arise on a real ostree system since the sysroot would
be locked during the freeze/thaw cycle.

However, when multiple fake sysroots are used during the test suite (run
as root), the tests could race to run the freeze/thaw cycle without
locking. Furthermore, there's no reason why an independent process might
be trying to freeze the filesystem while ostree was deploying. Ignore
but warn for these errors since there's not much ostree can do about it,
anyways.

Closes: #1260
Approved by: cgwalters

8 years agolib: Add a lighter weight internal checksum wrapper
Colin Walters [Fri, 6 Oct 2017 20:38:08 +0000 (16:38 -0400)]
lib: Add a lighter weight internal checksum wrapper

The faster (OpenSSL/GnuTLS) code lived in a `GInputStream` wrapper, and that
adds a lot of weight (GObject + vtable calls). Move it into a simple
autoptr-struct wrapper, and use it in the metadata path, so we're
now using the faster checksums there too.

This also drops a malloc there as the new API does hexdigest in place to a
buffer.

Prep for more work in the commit path to avoid `GInputStream` for local file
commits, and ["adopting" files](https://github.com/ostreedev/ostree/pull/1255).

Closes: #1256
Approved by: jlebon

8 years agocommit: Add _CONSUME modifier flag
Colin Walters [Tue, 3 Oct 2017 21:34:06 +0000 (17:34 -0400)]
commit: Add _CONSUME modifier flag

For many cases of commit, we can actually optimize things by simply "adopting"
the object rather than writing a new copy. For example, in rpm-ostree package
layering.

We can only make that optimization though if we take ownership of the file. This
commit hence adds an API where a caller tells us to do so. For now, that just
means we `unlink()` the files/dirs as we go, but we can now later add the
"adopt" optimization.

Closes: #1255
Approved by: jlebon

8 years agolib/deltas: Move variant read helper into the deltas code
Colin Walters [Thu, 5 Oct 2017 15:14:17 +0000 (11:14 -0400)]
lib/deltas: Move variant read helper into the deltas code

What the deltas code is doing is weird/unfortunate.  The name
`ot_variant_read()` conflicts too much with `ot_variant_read_fd()`.
Since nothing else uses it, move it into the deltas code.

Closes: #1254
Approved by: jlebon

8 years agolib/util: Delete some unused functions
Colin Walters [Wed, 4 Oct 2017 20:55:53 +0000 (16:55 -0400)]
lib/util: Delete some unused functions

Hooray, dead code.

Closes: #1254
Approved by: jlebon

8 years agolib/pull: Change fetcher to return O_TMPFILE
Colin Walters [Tue, 3 Oct 2017 01:36:10 +0000 (21:36 -0400)]
lib/pull: Change fetcher to return O_TMPFILE

A lot of the libostree code is honestly too complex for its
own good (this is mostly my fault).  The way we do HTTP writes
is still one of those.  The way the fetcher writes tempfiles,
then reads them back in is definitely one of those.

Now that we've dropped the "partial object" bits in:
https://github.com/ostreedev/ostree/pull/1176 i.e. commit
https://github.com/ostreedev/ostree/commit/0488b4870e80ef575d8b0edf6f2a9e5ad54bf4df
we can simplify things a lot more by having the fetcher
return an `O_TMPFILE` rather than a filename.

For trusted archive mirroring, we need to enable linking
in the tmpfiles directly.

Otherwise for at least content objects they're compressed, so we couldn't link
them in. For metadata, we need to do similar logic to what we have around
`mmap()` to only grab a tmpfile if the size is large enough.

Closes: #1252
Approved by: jlebon

8 years agolib/utils: Port a bit to decl-after-stmt style
Colin Walters [Wed, 4 Oct 2017 10:32:10 +0000 (06:32 -0400)]
lib/utils: Port a bit to decl-after-stmt style

Add add some more comments.

Closes: #1247
Approved by: jlebon

8 years agolib/repo: Clarify that ostree_repo_remote_fetch_summary() doesn’t verify
Philip Withnall [Thu, 5 Oct 2017 10:14:41 +0000 (11:14 +0100)]
lib/repo: Clarify that ostree_repo_remote_fetch_summary() doesn’t verify

Make that a bit clearer in the documentation.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1253
Approved by: cgwalters

8 years agoDeduplicate and fix up our use of mmap()
Colin Walters [Wed, 4 Oct 2017 19:06:31 +0000 (15:06 -0400)]
Deduplicate and fix up our use of mmap()

Buried in this large patch is a logical fix:

```
-  if (!map)
-    return glnx_throw_errno_prefix (error, "mmap");
+  if (map == (void*)-1)
+    return glnx_null_throw_errno_prefix (error, "mmap");
```

Which would have helped me debug another patch I was working
on.  But it turns out that actually correctly checking for
errors from `mmap()` triggers lots of other bugs - basically
because we sometimes handle zero-length variants (in detached
metadata).  When we start actually returning errors due to
this, things break.  (It wasn't a problem in practice before
because most things looked at the zero size, not the data).

Anyways there's a bigger picture issue here - a while ago
we made a fix to only use `mmap()` for reading metadata from disk
only if it was large enough (i.e. `>16k`).  But that didn't
help various other paths in the pull code and others that were
directly doing the `mmap()`.

Fix this by having a proper low level fs helper that does "read all data from
fd+offset into GBytes", which handles the size check. Then the `GVariant` bits
are just a clean layer on top of this. (At the small cost of an additional
allocation)

Side note: I had to remind myself, but the reason we can't just use
`GMappedFile` here is it doesn't support passing an offset into `mmap()`.

Closes: #1251
Approved by: jlebon

8 years agolib/commit: minor coverity fix
Jonathan Lebon [Wed, 4 Oct 2017 14:41:15 +0000 (14:41 +0000)]
lib/commit: minor coverity fix

Appease Coverity by using the same condition for both the ternary check
and the if-condition later on. It should be smart enough to figure out
that `dir_enum == NULL` implies that `dfd_iter != NULL` from the
assertion at the top of the function.

Coverity CID: #1457318

Closes: #1250
Approved by: cgwalters

8 years agolib/sysroot: Fix pointer going out of scope in unlock code
Colin Walters [Wed, 4 Oct 2017 14:24:21 +0000 (10:24 -0400)]
lib/sysroot: Fix pointer going out of scope in unlock code

Fixes Coverity CID #1457317

Closes: #1249
Approved by: jlebon

8 years agolib/core: Init struct stat buffer
Colin Walters [Wed, 4 Oct 2017 14:22:05 +0000 (10:22 -0400)]
lib/core: Init struct stat buffer

Regression from d57410a7e62dcb89321807dcb2d91c85f9d26df7

Fixes Coverity CID #1457316

Closes: #1249
Approved by: jlebon

8 years agotree: fix compiler warnings
Jonathan Lebon [Tue, 3 Oct 2017 21:34:43 +0000 (21:34 +0000)]
tree: fix compiler warnings

Mostly innocuous warnings, except for -Wtautological-compare, which
caught a shady guint64 subtraction.

Closes: #1245
Approved by: cgwalters

8 years agoci: really turn on warnings
Jonathan Lebon [Tue, 3 Oct 2017 21:34:43 +0000 (21:34 +0000)]
ci: really turn on warnings

We didn't have `-Wall` in our `CFLAGS`. It's normally injected by
`configure.ac`, but because we *did* have `-Werror`, it was skipped.
Now, we just turn it on unconditionally directly in `build.sh`.

Closes: #1245
Approved by: cgwalters

8 years agodocs: Clarify %NULL argument to ostree_sysroot_new()
Philip Withnall [Wed, 4 Oct 2017 11:17:31 +0000 (12:17 +0100)]
docs: Clarify %NULL argument to ostree_sysroot_new()

It’s equivalent to ostree_sysroot_new_default().

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1246
Approved by: cgwalters

8 years agoapp/main: use HAVE_LIBCURL_OR_LIBSOUP for pull CLI
Jonathan Lebon [Tue, 3 Oct 2017 19:00:32 +0000 (19:00 +0000)]
app/main: use HAVE_LIBCURL_OR_LIBSOUP for pull CLI

We want `pull` to be included as long as we have at least either
`libcurl` or `libsoup` to back it. Of course, this is a moot point for
now since `libsoup` is currently a build requirement.

Closes: #1244
Approved by: cgwalters

8 years agolib/repo-commit: Import detached metadata even if hardlink exists
Philip Withnall [Tue, 3 Oct 2017 14:45:34 +0000 (15:45 +0100)]
lib/repo-commit: Import detached metadata even if hardlink exists

Spotted while reading through the code, it looks like the
copy_detached_metadata() call is accidentally omitted if a hardlink
already exists for the .commit object.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1242
Approved by: cgwalters

8 years agolib/gpg: Print debug info when reading GPG keys
Matthew Leeds [Tue, 3 Oct 2017 05:35:17 +0000 (22:35 -0700)]
lib/gpg: Print debug info when reading GPG keys

This commit adds debug output whenever libostree reads GPG keys, which
can come from different locations in the file system. This is especially
helpful in debugging "GPG signatures found, but none are in trusted
keyring" errors, which in my case was caused by OSTree looking in
/usr/local/share/ostree/trusted.gpg.d/ rather than
/usr/share/ostree/trusted.gpg.d/.

Closes: #1241
Approved by: cgwalters

8 years agolib/pull: Minor cleanup to metadata scanning function, add docs
Colin Walters [Mon, 2 Oct 2017 19:36:47 +0000 (15:36 -0400)]
lib/pull: Minor cleanup to metadata scanning function, add docs

I'm regretting a bit having the `guint8*csum` variant of checksums
except for the serialized form.  Once we start doing processing
it's easier to just have it remain hex.

Do an on-stack conversion for the metadata scanning function; this
drops a malloc and also just looks nicer.

Also add some long-awaited function comments to the two.

Closes: #1240
Approved by: jlebon

8 years agolib/bloom: Add some missing preconditions on n_bytes
Philip Withnall [Mon, 2 Oct 2017 17:04:37 +0000 (18:04 +0100)]
lib/bloom: Add some missing preconditions on n_bytes

These shouldn’t change the bloom filter’s behaviour at all, but make it
a bit more obvious what the programmatical limitations are on the sizes
it can deal with.

In reality, those sizes should never be reached because they won’t fit
in a DNS-SD record.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1239
Approved by: cgwalters

8 years agolib/uboot: Port to fd-relative
Colin Walters [Sun, 1 Oct 2017 20:34:30 +0000 (16:34 -0400)]
lib/uboot: Port to fd-relative

Prep for `ostree_sysroot_new_at()`.

Closes: #1237
Approved by: jlebon

8 years agolib/syslinux: Port to fd-relative
Colin Walters [Sun, 1 Oct 2017 17:52:15 +0000 (10:52 -0700)]
lib/syslinux: Port to fd-relative

Prep for `ostree_sysroot_new_at()`.  Also just generally nicer.

Closes: #1237
Approved by: jlebon

8 years agolib/pull: Fix regression with pull-local for nonexistent refs
Colin Walters [Mon, 2 Oct 2017 15:24:05 +0000 (11:24 -0400)]
lib/pull: Fix regression with pull-local for nonexistent refs

I was reading the pull code for the last release, and spotted
a bug in commit f923c2e1eaebe0c781f07d34ae1a03f94357bccd - in
the case where the ref doesn't exist, we don't set an error,
tripping an assertion in the main code.

The previous code wanted the ref to always exist, so just flip back the boolean
for "ignore noent". I moved the `g_strchomp()` just into the HTTP path - if a
local repo is corrupted in this way it's something to fix in that repo.

Closes: #1238
Approved by: pwithnall

8 years agolib/sysroot: Add some more comments
Colin Walters [Fri, 29 Sep 2017 21:20:10 +0000 (17:20 -0400)]
lib/sysroot: Add some more comments

Also avoid gtk-doc style for private functions, as it tries to parse them and
complains since they aren't public.

Closes: #1230
Approved by: jlebon

8 years agolib/commit: Add some gtk-doc and internal doc comments
Colin Walters [Fri, 29 Sep 2017 19:31:03 +0000 (15:31 -0400)]
lib/commit: Add some gtk-doc and internal doc comments

Just making more of an effort for this for obvious reasons. We had a few public
APIs not documented too.

Closes: #1230
Approved by: jlebon

8 years agobuild-sys: Post-release version bump
Jonathan Lebon [Mon, 2 Oct 2017 14:48:03 +0000 (14:48 +0000)]
build-sys: Post-release version bump

Closes: #1236
Approved by: cgwalters

8 years agoRelease 2017.12
Colin Walters [Mon, 2 Oct 2017 14:21:27 +0000 (10:21 -0400)]
Release 2017.12

Closes: #1234
Approved by: jlebon

8 years agoostree/summary: Generate an ostree-metadata ref when updating summary
Philip Withnall [Mon, 11 Sep 2017 13:42:52 +0000 (14:42 +0100)]
ostree/summary: Generate an ostree-metadata ref when updating summary

This is the new way of publishing repository metadata, rather than as
additional-metadata in the summary file. The use of an ostree-metadata
ref means that the metadata from multiple upstream collections is not
conflated when doing P2P mirroring of many repositories.

The new ref is only generated if the repository has a collection ID set.
The old summary file continues to be generated for backwards
compatibility (and because it continues to be the canonical ref →
checksum map for the repository).

The new code is only used if configured with --enable-experimental-api.

Includes unit tests.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1158
Approved by: cgwalters

8 years agoostree/summary: Eliminate redundant gotos from error handling path
Philip Withnall [Mon, 11 Sep 2017 13:42:07 +0000 (14:42 +0100)]
ostree/summary: Eliminate redundant gotos from error handling path

There is no error handling to do, so just return everywhere instead.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1158
Approved by: cgwalters

8 years agoman: Fix a copypasta error in ostree-summary.xml
Philip Withnall [Mon, 11 Sep 2017 14:35:48 +0000 (15:35 +0100)]
man: Fix a copypasta error in ostree-summary.xml

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1158
Approved by: cgwalters

8 years agolib/bloom: Fix a -Wconversion warning in OstreeBloom
Philip Withnall [Sat, 30 Sep 2017 23:13:17 +0000 (00:13 +0100)]
lib/bloom: Fix a -Wconversion warning in OstreeBloom

Compiling with -Wconversion warns on this line, as the conversion from
guint64 to guint8 is implicit (but safe: there is no bug here, since the
implicit cast is applied after the modulus arithmetic).

Make the cast explicit to silence -Wconversion.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1231
Approved by: cgwalters

8 years agolib/bloom: Fix bloom hashing on 32-bit architectures
Philip Withnall [Sat, 30 Sep 2017 23:12:25 +0000 (00:12 +0100)]
lib/bloom: Fix bloom hashing on 32-bit architectures

There was an implicit cast from guint64 to gsize (which is 32-bit on
armhf, for example) before the modulus arithmetic which safely narrows
the index.

Fix that by using a guint64 intermediate variable and making the cast
explicit.

Signed-off-by: Philip Withnall <withnall@endlessm.com>
Closes: #1231
Approved by: cgwalters

8 years agotests/installed: also run test-basic-c
Jonathan Lebon [Fri, 29 Sep 2017 21:42:23 +0000 (21:42 +0000)]
tests/installed: also run test-basic-c

Since we now have a subtest there that needs full xattr support.

Closes: #1170
Approved by: cgwalters

8 years agotests: check for relabeling rather than overlay
Jonathan Lebon [Thu, 28 Sep 2017 18:57:33 +0000 (18:57 +0000)]
tests: check for relabeling rather than overlay

Instead of checking for overlayfs, let's explicitly check for our
ability to relabel files since we now have a `libtest` function to do
this. Also port that logic to `libostreetest`.

Note that overlayfs *does* allow manipulating user xattrs. So ideally,
we should break down `OSTREE_NO_XATTRS` further to distinguish between
tests that use bare repos from other modes.

We check the current directory instead of `/` so that developers can
just point `TEST_TMPDIR` to a non-overlayfs mount point when hacking
from a container.

Closes: #1170
Approved by: cgwalters

8 years agolib/commit: don't query devino cache for modified files
Jonathan Lebon [Thu, 28 Sep 2017 19:08:06 +0000 (19:08 +0000)]
lib/commit: don't query devino cache for modified files

We can't use the cache if the file we want to commit has been modified
by the client through the file info or xattr modifiers. We would
prematurely look into the cache in `write_dfd_iter_to_mtree_internal`,
regardless of whether any filtering applied.

We remove that path there, and make sure that we only use the cache if
there were no modifications. We rename the `get_modified_xattrs` to
`get_final_xattrs` to reflect the fact that the xattrs may not be
modified.

One tricky bit that took me some time was that we now need to store the
st_dev & st_ino values in the GFileInfo because the cache lookup relies
on it. I'm guessing we regressed on this at some point.

This patch does slightly change the semantics of the xattr callback.
Previously, returning NULL from the cb meant no xattrs at all. Now, it
means to default to the on-disk state. We might want to consider putting
that behind a flag instead. Though it seems like a more useful behaviour
so that callers can only override the files they want to without losing
original on-disk state (and if they don't want that, just return an
empty GVariant).

Closes: #1165
Closes: #1170
Approved by: cgwalters