Repository navigation
package: build each remote repository once per architecture - #295
Closed
martinpitt wants to merge 5 commits into
Closed
martinpitt wants to merge 5 commits into
martinpitt wants to merge 5 commits into
Conversation
Only boxes currently use this, but we are about to do the same for remote repositories. Signed-off-by: Martin Pitt <martin@amutable.com>
A remote repository used its target name as its id, carried to lock entries and the snapshot spec. The next commit declares one target per architecture, such as `arch.rolling.core.repository.x86_64`, so a repository's target name stops being unique. Pass the id as an explicit attribute instead. Signed-off-by: Martin Pitt <martin@amutable.com>
The next commit moves remote repositories to the execution configuration, which no longer says what architecture a caller needs. Prepare for that by declaring one target per served architecture with that architecture as an attribute, and give the repository's name to an alias that picks the caller's. The alias reaches them as plain dependencies still, so there is no effective change yet. Signed-off-by: Martin Pitt <martin@amutable.com>
The "repo matches package system" check previously compared the configured labels. The next commit moves repositories to be an `exec_dep`, so their target configuration will change and break this check. This was only ever meant to compare e.g. `rpm:package_system` to `pacman:package_system`, which is what `raw_target()` provides (the unconfigured target name). Signed-off-by: Martin Pitt <martin@amutable.com>
A box lock, the box and an image each reach a repository in their own configuration, so each of these downloaded every repo file again. To avoid that, reach the target of each architecture through an execution dependency, same pattern as commit 20176b6 did for boxes. Signed-off-by: Martin Pitt <martin@amutable.com>
Member
Author
|
Let's go with #300 and shelve this. |
This file contains hidden or 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
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 box lock, the box and an image each reach a repository in their own configuration, so each of these downloaded every repo file again. To avoid that, reach the target of each architecture through an execution dependency, same pattern as commit 20176b6 did for boxes.
This is quite a hairy issue, see #292 . There are other ways to fix this, but I chose the pattern that we already do for boxes. It's just annoyingly noisy.
Sadly,
what-randoes not show downloads, and as downloads happen in parallel, this is hard to measure with execution times. But it can be demoed like this. With this branch:On current main, that command fails, and already downloads more than the above, and didn't even download everything yet before it crashes:
The tar error is because of a Buck2 bug: multiple parallel
download_files()are not atomic, and write into the same file. I sent facebook/buck2#1561 for that, but I want to avoid downloading every repo file and rpm/pkg/deb three times.Fixes #292