ostree
ostree copied to clipboard
Enhance command line utils to take a tree/file sha
...in place of a commit sha where reasonable.
TODO
- [x] Remove debug prints
- [x] Remove private API from symbol table
- [x] Fix code formatting WRT space between function name and argument list
- [x] Make CI pass
- ~~Comment on "000000000000000000000000" SHA check~~ 000...000 removed. I replaced it with
dirmeta_0755_0_0 - [ ] Use
g_file_query_infoin place ofg_file_query_file_type - [ ] Use
g_assert (OSTREE_IS_REPO_FILE (root_gfile)); - [x] Add more
0s to dummy SHA or replace it with something else. - replaced - [ ] Fix bug passing a single
ref:filenameargument toostree diff
Description
The motivation for this is to enhance ostree commit --tree=ref to allow composing subtrees. For example: in our build system we strip away everything not under /usr as the last step when building containers. This PR (in combination with #1651) allows you to do:
ostree commit --tree=prefix=/usr/lib/machines/container/usr --tree=ref=:container:usr
To do this. It's fast.
As I was going along I thought it would be useful to have this ability generally in the command line tools. So now many of them accept a treeish instead of a commit.
Previously these tools accepted a COMMIT as:
- REF
- REMOTE:REF
- COMMIT_SHA
And now in addition:
- REMOTE:REF:PATH
- :REF:PATH
- COMMIT_SHA:PATH
- TREE_SHA:PATH
So the idea is that you append :PATH to a commit and you can select a tree. This is the same as the git syntax. As a ref may already contain a colon if the REMOTE is specified we disambiguate by requiring the remote to be specified, even if it is left empty.
Offhand the idea is cool, and I love to help push forward the container+OS as single tree concept and make that work better.
It's not a private API if we're adding it to the symbol table :smile:. We did add a special way for the CLI to call into private library bits, see ostree-cmdprivate.[ch]. That's probably a good approach until we decide to make the API stable?
In just glancing at the code: There are various leftover debug prints, and can you fix the space-between-identifer-and-parent i.e. g_strcmp0 ( vs g_strcmp0(.
Thanks again for working on this! Really looking forward to getting more of your patches in.
:umbrella: The latest upstream changes (presumably c7b12a8) made this pull request unmergeable. Please resolve the merge conflicts.
In just glancing at the code: There are various leftover debug prints, and can you fix the space-between-identifer-and-parent i.e. g_strcmp0 ( vs g_strcmp0(.
Sorry, that was sloppy. I was in a rush to finish it the week before last as I suspected that I wouldn't have time last week.
I've fixed the formatting issues - at least the ones I spotted. I had a crack at configuring GNU indent and clang-format to format according to ostree style, but failed on both. I couldn't get either to align the * in function definitions correctly. I think https://reviews.llvm.org/D27651 is the clang bug.
I've changed the approach to private API as recommended. We use ostree-cmdprivate.h now.
I've also broken up the main commit into 3 to make it easier to review. ~~CI passes too.~~
Please review.
CI isn't passing, but I don't think it's related:
f28-rust says:
Error: Error downloading packages: Status code: 503 for https://mirrors.fedoraproject.org/metalink?repo=updates-released-f28&arch=x86_64
FAH28-insttests says:
Could not resolve host: kojipkgs.fedoraproject.org
There's a bug in ostree diff introduced here. Running:
ostree --repo=/ostree ls :stbt-node-rootfs/b51ca22876b3b7ff776870bca1e8c1ff92e6bcbb:usr/bin
fails with
error: Bad ref ":stbt-node-rootfs/0b791b5379ea56d111307f112b2bce0cc5a2d732:usr/bin^": "bin^": No such file or directory
presumably because ostree diff by default adds ^ to the ref to get the previous ref. I'll need to add a test and fix this.
:umbrella: The latest upstream changes (presumably 6869bad) made this pull request unmergeable. Please resolve the merge conflicts.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: wmanley
To complete the pull request process, please assign jlebon
You can assign the PR to them by writing /assign @jlebon in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
@wmanley: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@wmanley: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:
| Test name | Commit | Details | Required | Rerun command |
|---|---|---|---|---|
| ci/prow/sanity | b4c51bceb14f402ea15be6a648ac5fbe04d3589b | link | true | /test sanity |
Full PR test history. Your PR dashboard.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.