treefile: Support arch-include
#1886
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Aug 20, 2019
The package is only present on `x86_64` and `aarch64` and previously we'd error out. Requires: coreos/rpm-ostree#1886
|
No tests yet, will do soon, but I did test this manually with coreos/fedora-coreos-config#149 |
jlebon
reviewed
Aug 20, 2019
docs/manual/treefile.md
Outdated
| object should be the name of an architecture, and the `include` value | ||
| functions the same as the `include` key above - it can be either | ||
| a single string, or an array of strings - and it has the same semantics. | ||
| `arch-include` is processed after `include`. |
There was a problem hiding this comment.
To be more explicit here, how about: "Entries which match the $basearch of the target platform are processed after the regular include" ?
Have each test clearly hold its tempdir; this way we can have a simple helper function rather than a struct with an unreferenced parameter. Also use `utils::write_file`. Prep for further test work.
Prep for adding an arch include test.
cgwalters
force-pushed
the
arch-includes
branch
2 times, most recently
from
e5fedcb to
69ea48e
Compare
|
Updated 🆕 with tests! |
|
@barthy1 @tuan-hoang1 and just in case @ausil @jcajka |
dustymabe
approved these changes
Aug 21, 2019
| functions the same as the `include` key above - it can be either | ||
| a single string, or an array of strings - and it has the same semantics. | ||
| Entries which match `arch-include` are processed after `include`. | ||
|
|
There was a problem hiding this comment.
could we get an example like is done below for add-files ? For me it really came together after I looked at the related FCOS PR
There was a problem hiding this comment.
Sure, done.
(Although of course bigger picture part of the idea here is that the FCOS config becomes also the reference treefile; we don't want most people writing them from scratch anyways)
A long time ago we added architecture-specific package lists via e.g. `packages-ppc64le`. Much more recently we added support for having the `include` key be a list - multiple includes. By combining these two and supporting architecture-conditional includes, we've effectively added architecture-conditionals to *all* keys. Notably we want this for Fedora CoreOS today which is using `remove-from-packages` on `grub2-tools` which isn't present on s390x.
cgwalters
force-pushed
the
arch-includes
branch
from
69ea48e to
606e3ee
Compare
|
📌 Commit 606e3ee has been approved by |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Have each test clearly hold its tempdir; this way we can have a simple helper function rather than a struct with an unreferenced parameter. Also use `utils::write_file`. Prep for further test work. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Prep for adding an arch include test. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
A long time ago we added architecture-specific package lists via e.g. `packages-ppc64le`. Much more recently we added support for having the `include` key be a list - multiple includes. By combining these two and supporting architecture-conditional includes, we've effectively added architecture-conditionals to *all* keys. Notably we want this for Fedora CoreOS today which is using `remove-from-packages` on `grub2-tools` which isn't present on s390x. Closes: #1886 Approved by: dustymabe
|
💔 Test failed - status-atomicjenkins |
|
@rh-atomic-bot retry |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Have each test clearly hold its tempdir; this way we can have a simple helper function rather than a struct with an unreferenced parameter. Also use `utils::write_file`. Prep for further test work. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Prep for adding an arch include test. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
A long time ago we added architecture-specific package lists via e.g. `packages-ppc64le`. Much more recently we added support for having the `include` key be a list - multiple includes. By combining these two and supporting architecture-conditional includes, we've effectively added architecture-conditionals to *all* keys. Notably we want this for Fedora CoreOS today which is using `remove-from-packages` on `grub2-tools` which isn't present on s390x. Closes: #1886 Approved by: dustymabe
Should we increase the timeout? |
|
This is probably another internal OpenStack performance regression. If it's taking an hour just to pull the container image, we'd need to massively bump the timeout. Really need to use bare metal machines for our CI consistently. |
|
💔 Test failed - status-atomicjenkins |
Looks like another OpenStack perf regression.
|
Well, that one got farther. Bumped to two hours. @rh-atomic-bot r=dustymabe ff55a0c |
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Have each test clearly hold its tempdir; this way we can have a simple helper function rather than a struct with an unreferenced parameter. Also use `utils::write_file`. Prep for further test work. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Prep for adding an arch include test. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
A long time ago we added architecture-specific package lists via e.g. `packages-ppc64le`. Much more recently we added support for having the `include` key be a list - multiple includes. By combining these two and supporting architecture-conditional includes, we've effectively added architecture-conditionals to *all* keys. Notably we want this for Fedora CoreOS today which is using `remove-from-packages` on `grub2-tools` which isn't present on s390x. Closes: #1886 Approved by: dustymabe
rh-atomic-bot
pushed a commit
that referenced
this pull request
Aug 21, 2019
Looks like another OpenStack perf regression. Closes: #1886 Approved by: dustymabe
|
💔 Test failed - status-atomicjenkins |
|
OK, at least the cosa test passed now, but other tests timed out. I'm force merging for now. |
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Aug 23, 2019
The package is only present on `x86_64` and `aarch64` and previously we'd error out. Requires: coreos/rpm-ostree#1886
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Aug 23, 2019
The package isn't present on s390x and previously we'd we'd error out. This is really a hack; I think the better fix would be to move that stuff out of the low-level `grub2-tools` package. Requires: coreos/rpm-ostree#1886
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Aug 26, 2019
The package isn't present on s390x and previously we'd we'd error out. This is really a hack; I think the better fix would be to move that stuff out of the low-level `grub2-tools` package. Requires: coreos/rpm-ostree#1886
33 tasks
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Sep 9, 2019
The package isn't present on s390x and previously we'd we'd error out. This is really a hack; I think the better fix would be to move that stuff out of the low-level `grub2-tools` package. Requires: coreos/rpm-ostree#1886
cgwalters
added a commit
to cgwalters/fedora-coreos-config
that referenced
this pull request
Sep 9, 2019
The package isn't present on s390x and previously we'd we'd error out. This is really a hack; I think the better fix would be to move that stuff out of the low-level `grub2-tools` package. Requires: coreos/rpm-ostree#1886
cgwalters
added a commit
to coreos/fedora-coreos-config
that referenced
this pull request
Sep 10, 2019
The package isn't present on s390x and previously we'd we'd error out. This is really a hack; I think the better fix would be to move that stuff out of the low-level `grub2-tools` package. Requires: coreos/rpm-ostree#1886
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long time ago we added architecture-specific package lists
via e.g.
packages-ppc64le. Much more recently we addedsupport for having the
includekey be a list - multiple includes.By combining these two and supporting architecture-conditional includes,
we've effectively added architecture-conditionals to all keys.
Notably we want this for Fedora CoreOS today which is using
remove-from-packagesongrub2-toolswhich isn't present ons390x.