Skip to content

Add arch to rhel fetch#491

Open
mostlikelee wants to merge 2 commits intovulsio:masterfrom
mostlikelee:add-rhel-arch
Open

Add arch to rhel fetch#491
mostlikelee wants to merge 2 commits intovulsio:masterfrom
mostlikelee:add-rhel-arch

Conversation

@mostlikelee
Copy link

If this Pull Request is work in progress, Add a prefix of “[WIP]” in the title.

What did you implement:

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

  • goval-dictionary fetch redhat 9
  • spot checked sqlite ensuring multiple rows with unique arch values

Checklist:

You don't have to satisfy all of the following.

  • Write tests
  • Write documentation
  • Check that there aren't other open pull requests for the same issue/feature
  • Format your source code by make fmt
  • Pass the test by make test
  • Provide verification config / commands
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES

Reference

@mostlikelee
Copy link
Author

@MaineK00n any thoughts on this PR?

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds architecture awareness to Red Hat OVAL parsing so affected packages are emitted/stored per-architecture (instead of collapsing across arches).

Changes:

  • Introduce an archResolver to map rpminfo_testrpminfo_state → arch text and emit packages per arch.
  • Update Red Hat package collection to de-duplicate/version-select per (name, arch) key.
  • Extend unit tests to cover multi-arch expansion and update comparisons.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
models/redhat/redhat.go Adds arch resolution and emits models.Package entries per architecture; de-dupes by (name, arch) during collection.
models/redhat/redhat_test.go Updates tests for new collectRedHatPacks signature and adds multi-arch test cases.
Comments suppressed due to low confidence (1)

models/redhat/redhat_test.go:350

  • The sorting comparator ignores Arch (and Version), so packages that differ only by Arch can end up in non-deterministic order (since collectRedHatPacks builds results from a map). This can make slices.Equal comparisons flaky. Include Arch (and any other differentiating fields, e.g. Version/NotFixedYet) in the sort key for both actual and tt.expected.
		sort.Slice(actual, func(i, j int) bool {
			if actual[i].Name == actual[j].Name {
				return actual[i].ModularityLabel < actual[j].ModularityLabel
			}
			return actual[i].Name < actual[j].Name
		})

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@MaineK00n
Copy link
Collaborator

MaineK00n commented Jan 23, 2026

@MaineK00n any thoughts on this PR?

I'm very busy at the moment, so I won't be able to see it right away.

I would like you to fill out the body of the PR and format the commit message by referring to other commits.
I think it's good to be able to take architecture.

@MaineK00n MaineK00n requested review from MaineK00n and shino January 23, 2026 19:36
@MaineK00n
Copy link
Collaborator

The "How Has This Been Tested?" section is insufficient. Please show that the appropriate arch is included, even if it is just the sqlite3 pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants