extra-software: Add per-arch config support - #1137
Merged
Merged
Conversation
kostyanf14
requested review from
elizashurov
and
a lite review from Copilot
September 14, 2026 13:26
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues affect architecture selection, staging, downloading, and installation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds per-architecture extra-software configuration support across Ruby preparation and PowerShell installation.
Changes:
- Supports architecture-specific config names and AMD64/x64 aliases.
- Resolves, downloads, and installs software per target architecture.
- Prepares configs for platform clients and studio installation.
File summaries
| File | Summary |
|---|---|
lib/engines/hckinstall/setup-scripts/extra_software.ps1 |
Adds architecture-specific config lookup. Moderate finding (1 vote): candidate ordering can select a different config than Ruby. |
lib/auxiliary/extra_software/manager.rb |
Adds architecture-aware preparation and installation. Moderate findings: fallback architectures may not be prepared (1 vote); rejected architectures may still be staged or installed (2 votes); unused platform architectures may trigger missing-config errors (1 vote); conflicting filenames can reuse the wrong download (1 vote). |
Review details
Suppressed comments (5)
lib/auxiliary/extra_software/manager.rb:192
machine_archresolves a client with no explicitarchtoplatform.client_archorProject::DEFAULT_ARCH, but this loop only prepares configs for clients whosearchis non-nil. On a valid mixed platform (one client has an explicit architecture and another relies on the fallback), the fallback client's config is never prepared and installation is skipped after the warning insw_config_for. Add the same fallback architecture here.
arches << client.arch if client.arch
lib/auxiliary/extra_software/manager.rb:193
platform_archesnow includes every declared client architecture, but Functest prepares extra software during engine initialization and boots only the union of client IDs selected by the chosen tests. On a mixed-architecture platform, a package needed by the selected x64 client but lacking an x86 config for an unused client will therefore raiseExtraSoftwareMissingConfigbefore the run starts. Limit this to architectures that the engine will actually prepare, or pass explicit architectures from the callers.
arches << platform.client_arch if platform.client_arch
platform.clients.each_value do |client|
arches << client.arch if client.arch
end
lib/auxiliary/extra_software/manager.rb:179
- The alias ordering is not consistent between the Ruby preparation path and the PowerShell runtime path: for platform arch
x64, Ruby checksx64beforeamd64, while a Windows AMD64 process checksamd64beforex64. If both alias-specific files exist, the package is prepared from one config but the VM installs using the other. Canonicalizex64toamd64(or use the same precedence in both implementations).
arch_name = arch.to_s.downcase
variants = [arch_name]
variants << 'x64' if arch_name == 'amd64'
variants << 'amd64' if arch_name == 'x64'
variants.uniq
lib/auxiliary/extra_software/manager.rb:95
- When two architectures use configs with the same
file_namebut different download URLs, the first architecture downloads<extra_software>/<name>/<file_name>and the second call sees that path already exists and skips its download. The second architecture will then install the first architecture's binary; use architecture-specific download paths or reject conflictingfile_name/URL pairs before downloading.
download_software(name, config)
lib/engines/hckinstall/setup-scripts/extra_software.ps1:40
- The PowerShell candidate order does not match
config_path_candidates: this interleaves<kit>-<arch>and<arch>files, so if (for example)amd64-config.jsonand<kit>-x64-config.jsonboth exist, the host loads the latter while the setup script loads the former. That can install a package using a differentfile_nameor command than the one downloaded. Search all kit/architecture variants before the architecture-only variants, as the Ruby code does.
foreach ($archVariant in (Get-ArchConfigVariants $arch)) {
$config_list += "${Directory}\${full_kit}-${archVariant}-config.json"
$config_list += "${Directory}\${archVariant}-config.json"
}
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
kostyanf14
force-pushed
the
VIRTWINKVM-2716
branch
from
September 14, 2026 13:55
3eef521 to
af7d6fd
Compare
elizashurov
approved these changes
Sep 14, 2026
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.
No description provided.