Add V2 depsolver API support and make it the default (HMS-10029)#2139
Merged
supakeen merged 13 commits intoosbuild:mainfrom Jan 26, 2026
Merged
Add V2 depsolver API support and make it the default (HMS-10029)#2139supakeen merged 13 commits intoosbuild:mainfrom
supakeen merged 13 commits intoosbuild:mainfrom
Conversation
thozza
force-pushed
the
HMS-9062
branch
3 times, most recently
from
999246d to
528b196
Compare
thozza
added a commit
to thozza/osbuild
that referenced
this pull request
Jan 19, 2026
osbuild/images#2139 Signed-off-by: Tomáš Hozza <thozza@redhat.com>
thozza
added a commit
to thozza/osbuild
that referenced
this pull request
Jan 19, 2026
osbuild/images#2139 Signed-off-by: Tomáš Hozza <thozza@redhat.com>
lzap
requested changes
Jan 22, 2026
Contributor
lzap
left a comment
lzap
left a comment
There was a problem hiding this comment.
Nicely done. Please add the documentation for the common helper.
Modify the unit tests to allow running them against list of apiHandler implementations. This will allow easily running the same unit tests against V2 depsolver API, once added. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
The depsolver results are sorted by "full" NEVRA, so let's add a convenient method to get it for a Package. The depsolver API implementations may use it for sorting results. Unit tests will also leverage this method. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Apply repo-specific values (RepoID, RemoteLocations, CheckGPG, IgnoreSSL) to package templates at assertion time, instead of duplicating them in each package definition. This reduces test data duplication and makes the expected package list easier to maintain. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
The V2 API will set much more fields in the rpmmd.Package instances returned in the depsolve result. To keep testing that the currently used fields are set by any tested API handler, compare only fields set by V1 API handler. Refactor the depsolve unit tests to allow specifying depsolve result assert function specific to the tested apiHandler implementation. This will allow us to have broader testing for V2 API handler later. Add assertDepsolveResultV1 which: - Accepts package sets instead of pre-built expected result. This will allow later to check specific Transaction results. - Builds expected packages internally from actual.Repos. The unified depsolve result package set is always the same in all scenarios. - Compares only "original" V1 API core fields (Name, Epoch, Version, Release, Arch, RepoID, Location, RemoteLocations, Checksum, CheckGPG, IgnoreSSL). - Verifies all requested packages are present in the result. The assertion signature prepares for V2 API transaction checking without requiring further signature changes. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Add a new Transactions field to DepsolveResult that contains a list of package lists, one per depsolve transaction. Each transaction contains only the packages unique to that transaction, representing disjoint sets that should be installed in order. This new field will be set by V2 depsolver API implementation and later used to generate rpm stages per transaction results in manifests. The existing Packages field is kept for backwards compatibility but is marked for future removal once all clients have been updated to use Transactions. Extend unit tests to assert that Transactions value is not set by the V1 API. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Add Repos and Solver fields to DumpResult and SearchResult structs to match the structure of DepsolveResult. These fields will be populated by the V2 depsolver API implementation, which returns repository metadata and solver information in dump and search responses. The V1 API handler will leave these fields as zero values (empty slice and empty string), which is safe for existing callers. Add comments noting that FetchMetadata and SearchMetadata should cache and expose the full operation result instead of just the packages in the future. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Include full metadata for the bash package (Group, License, SourceRpm, BuildTime, Provides, Requires, Files, etc.) to enable more comprehensive V2 API testing later. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Add a generic ClonePtr[T] function that creates an independent copy of a pointer value. Unlike direct pointer assignment which creates aliased pointers (where modifications to one affect the other), ClonePtr dereferences the source pointer and returns a pointer to a fresh copy. The function safely handles nil pointers by returning nil, eliminating the need for nil checks at call sites. This complements the existing ToPtr helper. While the pattern ToPtr(*ptr) achieves the same result, ClonePtr(ptr) is more self-documenting and makes the intent of creating a deep copy explicit. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Replace the ToPtr(*ptr) pattern with the new ClonePtr helper function for safely copying pointer values. Since ClonePtr handles nil pointers internally by returning nil, this also removes redundant nil checks at call sites, resulting in cleaner and more concise code. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
This ensures that the minimum osbuild version is the one that contains depsolver V2 API. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Add apiHandler interface implementation for the V2 depsolver API. The V2 provides richer package metadata than V1 and also provides per-transaction package results. The V2 API is no longer as strict as V1 implementation and does not fail on unknown fields in the depsolver reply. This has been a source of a lot of pain in the past. Although we now have a much nicer and easier way to iterate on depsolver API changes, introducing a new API version just to add a new field is probably an overkill and we should not make this unnecessarily hard. Populate Repos and Solver fields in DumpResult and SearchResult from the V2 API response. This provides the API callers with repository metadata and solver information from dump and search operations. Add comprehensive unit tests for the V2 handler: - Extract assertExpectedPackages() as a shared helper between V1 and V2 assertions to verify packages match expected using V1 core fields and that all requested packages are present in the result. - Add assertDepsolveResultV2() which performs V2-specific checks: - Verifies Packages field (for backwards compatibility) - Checks full metadata for bash package as a smoke test - Verifies transaction count matches the number of package sets - Ensures each requested package appears in the correct transaction - Validates that the union of all transactions equals expected packages The V2 handler is not used anywhere yet. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
Switch the Solver to use the V2 depsolver API by default. The V1 API implementation is kept around for the sake of being tested in CI via unit tests and proving that things keep working. V1 will be completely removed later, since it is not expected to be use going forward. Signed-off-by: Tomáš Hozza <thozza@redhat.com>
lzap
approved these changes
Jan 23, 2026
Contributor
lzap
left a comment
lzap
left a comment
There was a problem hiding this comment.
I am not fan of this naked ClonePtr for cloning because chances are we miss some fields and then hunting a shared memory bug can be costly. While in Go the manual copying field by field is the standard, there are libraries which are decently fast and can do copying for us, like copier.
Definitely something not for this PR, but this is the first time I see copying at scale in this repo and maybe this could help to create cleaner code.
Anyways, everything is clean, I haven't run the code yet.
Member
Author
I agree. Go makes it easy to be lazy with pointers. It’s convenient enough that you often don't think about memory layout, which leads to accidental shared memory and subtle bugs. Using a library to handle deep cloning, rather than implementing it manually for each structure, would definitely be nicer. |
supakeen
approved these changes
Jan 26, 2026
Member
supakeen
left a comment
supakeen
left a comment
There was a problem hiding this comment.
I have a small question unrelated to the code here but how large do our depsolve results get now, in bytes?
Member
Author
I don't have the exact number, I can test it. But the general answer is "considerably larger" due to all the dependencies, provides, files lists, etc... 😬 |
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
3 participants
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.
Add V2 depsolver API support with complete package metadata, per-transaction results, and RHSM secrets discovery. Make it the default.
Overview
This PR introduces V2 depsolver API bindings and switches the images library to use them by default. The V2 API provides richer package metadata and per-transaction depsolve results, which will enable generating per-transaction RPM osbuild stages in future work. Specifically to resolve Image build failed due to package pre trans scripts issue. The V1 API is retained for CI unit testing to prove behavioral equivalence. All public APIs remain backward compatible—existing clients using the
Packagesfield continue to work unchanged.Architectural Changes
The V2 handler is more lenient with unknown JSON fields in depsolver responses, reducing maintenance burden when the depsolver adds new fields. A new
Transactionsfield inDepsolveResultexposes per-transaction package lists (disjoint sets), while the existingPackagesfield is preserved for backward compatibility. TheDumpResultandSearchResultstructs now includeReposandSolverfields populated by V2 responses.Key Changes
v2.gowith complete V2 API request/response structures and handler implementationTransactionsfield toDepsolveResultcontaining per-transaction package listsReposandSolverfields toDumpResultandSearchResultstructsClonePtr[T]helper function for safe pointer copying, replacingToPtr(*ptr)pattern and use it inv2.goBreaking Changes
This PR is fully backward compatible. The
Packagesfield remains populated and unchanged for all existing callers. V1 API code is retained but only exercised in unit tests.Testing
DepsolveResultassertion helper (assertDepsolveResultV2) that verifies transaction structure, per-transaction package placement, and full metadata parsing