Skip to content

repos: force data is an override#388

Merged
thozza merged 2 commits intoosbuild:mainfrom
supakeen:force-data-actually-force-data
Nov 28, 2025
Merged

repos: force data is an override#388
thozza merged 2 commits intoosbuild:mainfrom
supakeen:force-data-actually-force-data

Conversation

@supakeen
Copy link
Member

@supakeen supakeen commented Nov 28, 2025

This changes the handling for --force-data-dir to actually override any built-in repositories instead of merely overlaying them. Overlaying can be introduced later as --extra-data-dir instead.

The behavior after this commit is the intended and expected behavior.

I've also fixed the deprecation warning being emitted for --force-data-dir and --data-dir which was found by @thozza. I couldn't really find a nice way to do it; so I've done the verbose way. If someone has a better idea let me know :)

@supakeen supakeen requested a review from a team as a code owner November 28, 2025 09:29
@supakeen supakeen requested review from bcl, croissanne, mvo5 and thozza and removed request for a team November 28, 2025 09:29
@supakeen supakeen force-pushed the force-data-actually-force-data branch 2 times, most recently from b907bad to 3d92d3a Compare November 28, 2025 09:45
lzap
lzap previously approved these changes Nov 28, 2025
], text=True)

# ensure we only have lines containing `rhel-10.0`
assert all("rhel-10.0" in line for line in output.splitlines())
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use non-existing-os-1.0 just to make sure the code does not actually load our built-in OS repos. I guess the OS must exist, then could you perhaps use an architecture that is no longer supported in RHEL 10 and check for that combination? Assuming this is possible, if not, ignore my comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

That would require also passing in custom definitions.

Copy link
Member

Choose a reason for hiding this comment

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

The fact that the built-in OS repos are not loaded is asserted by checking that no other than rhel-10.0 distributions are shown as available. If we wanted to be fancy, we could also test that only x86_64 is available for that distro.

Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thanks, the repos commit looks good (once small suggestion about the test only). The other one I would like to look at some more, maybe worth splitting this PR if you want to land this quicker?

The --data-dir is deprecated in favor of --force-data-dir.
@supakeen supakeen force-pushed the force-data-actually-force-data branch from 5af3721 to 8a9c1b8 Compare November 28, 2025 10:40
With the rename of `data-dir` to `force-data-dir` it is implied that
only data within the `force-data-dir` is being used. However, the
repositories were always merged with the built-in embedded filesystem.

This commit makes it so that when a `force-data-dir` is passed as an
argument then only the repositories inside that directory are used.

Note that the repository files inside the data directory still must
match any defined distributions in the definitions.

Also add a small smoke test that uses `force-data-dir` to only set up a
`rhel-10.0` repository and assert that there are only listed images for
`rhel-10.0`.

Signed-off-by: Simon de Vlieger <supakeen@redhat.com>
@supakeen supakeen force-pushed the force-data-actually-force-data branch from 8a9c1b8 to 8305290 Compare November 28, 2025 10:46
Copy link
Contributor

@mvo5 mvo5 left a comment

Choose a reason for hiding this comment

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

Thank you

Copy link
Member

@thozza thozza left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thozza thozza enabled auto-merge November 28, 2025 10:50
@thozza thozza added this pull request to the merge queue Nov 28, 2025
Merged via the queue into osbuild:main with commit 97de1d2 Nov 28, 2025
38 checks passed
@supakeen supakeen deleted the force-data-actually-force-data branch November 28, 2025 12:14
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.

4 participants