Skip to content

Augment repository files with default priority when missing#4454

Merged
thrix merged 2 commits intomainfrom
vaibhav-augment-priority
Jan 14, 2026
Merged

Augment repository files with default priority when missing#4454
thrix merged 2 commits intomainfrom
vaibhav-augment-priority

Conversation

@vaibhavdaren
Copy link
Copy Markdown
Contributor

@vaibhavdaren vaibhavdaren commented Dec 29, 2025

  1. User-provided repository files (via repository-file provider): These are treated as authoritative user input and installed verbatim. If the .repo file defines a priority, it is preserved; if not, no priority is added.

  2. tmt-artifact-shared repository: This is an internally-managed repository where downloaded artifacts are aggregated. It has a higher priority (lower numeric value) to have precedence over system repositories.

@vaibhavdaren vaibhavdaren added the plugin | artifact Related to the `prepare/artifact` plugin. label Jan 2, 2026
@LecrisUT LecrisUT self-assigned this Jan 5, 2026
@vaibhavdaren vaibhavdaren marked this pull request as ready for review January 5, 2026 12:41
@github-project-automation github-project-automation Bot moved this to backlog in planning Jan 6, 2026
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-augment-priority branch from 9c14d2a to 02b2c96 Compare January 6, 2026 13:54
@psss psss added this to the 1.65 milestone Jan 8, 2026
@vaibhavdaren vaibhavdaren moved this from backlog to implement in planning Jan 8, 2026
@vaibhavdaren vaibhavdaren moved this from implement to review in planning Jan 11, 2026
@vaibhavdaren vaibhavdaren requested review from LecrisUT and psss and removed request for psss January 12, 2026 08:34
@vaibhavdaren vaibhavdaren added the ci | full test Pull request is ready for the full test execution label Jan 12, 2026
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-augment-priority branch from 02b2c96 to ab0fc70 Compare January 12, 2026 08:37
@happz
Copy link
Copy Markdown
Contributor

happz commented Jan 12, 2026

I'm not sure we should be touching repositories in this way, especially repositories supplied as repo files. If they do not have priority set, are we sure it's a mistake? And that 50 is the right value to set? It would make sense to allow user to specify a priority if they wished, e.g. by supporting something like provide: repository-url:https://.../foo.repo:priority=50, but forcing single priority (which users cannot change...) does not seem good to me.

What is the motivation behind the change and the actual values?

@vaibhavdaren vaibhavdaren added the status | discuss Needs more discussion before closing label Jan 12, 2026
@vaibhavdaren
Copy link
Copy Markdown
Contributor Author

vaibhavdaren commented Jan 12, 2026

Context: Assume package P1 exists in two repositories:
R1, which is a repository explicitly added via the artifact install plugin, and
R2, which is an existing system/base repository.
If R1’s repo file does not define a priority, DNF treats it as having the default priority 99, the same as R2. When running dnf install P1, DNF may then resolve P1 from R2 instead of R1, even though R1 was explicitly added for this operation.

Q1-> What should dnf resolve to R1 or R2. [We need to understand the default behaviour of DNF]
Q2-> Does this behaviour for dnf change over different os versions [ We need to alligin with the team so that dependency resolution is deterministic]

Expected / desired behavior
When a user adds a repository explicitly (R1), the intuitive expectation is that packages should be installed from that repository unless the user has clearly indicated otherwise. In the example above, that means P1 should come from R1, not R2, because the act of adding R1 signals intent to use it.

What we are trying to do
By assigning a non-default priority (e.g. 50) to the provided repository, we ensure that R1 is preferred over default system repositories (priority 99) during dependency resolution. This makes the behavior deterministic and avoids surprising outcomes where packages silently come from R2.

The exact value is not special; it simply needs to be better than the default, and this can later be extended to allow users to specify a custom priority if needed as you told.

@happz
Copy link
Copy Markdown
Contributor

happz commented Jan 13, 2026

Context: Assume package P1 exists in two repositories: R1, which is a repository explicitly added via the artifact install plugin, and R2, which is an existing system/base repository. If R1’s repo file does not define a priority, DNF treats it as having the default priority 99, the same as R2. When running dnf install P1, DNF may then resolve P1 from R2 instead of R1, even though R1 was explicitly added for this operation.

Q1-> What should dnf resolve to R1 or R2. [We need to understand the default behaviour of DNF] Q2-> Does this behaviour for dnf change over different os versions [ We need to alligin with the team so that dependency resolution is deterministic]

Expected / desired behavior When a user adds a repository explicitly (R1), the intuitive expectation is that packages should be installed from that repository unless the user has clearly indicated otherwise. In the example above, that means P1 should come from R1, not R2, because the act of adding R1 signals intent to use it.

What we are trying to do By assigning a non-default priority (e.g. 50) to the provided repository, we ensure that R1 is preferred over default system repositories (priority 99) during dependency resolution. This makes the behavior deterministic and avoids surprising outcomes where packages silently come from R2.

The exact value is not special; it simply needs to be better than the default, and this can later be extended to allow users to specify a custom priority if needed as you told.

Not convinced it should be us who slaps a hardwired priority on user-provided repository file, sorry. Why can't we tell user "hey. your repo lacks priority, you're on your own - you want predictability? add priority key, FYI, system repos have 99, 50 would work well". User gives us a file and says "this I need to be added to the system", and we wouldn't do that, we add some modified version of it.

@vaibhavdaren
Copy link
Copy Markdown
Contributor Author

vaibhavdaren commented Jan 13, 2026

Yes. that’s exactly the right consistency model, and I think we should make it explicit.

Proposed rule

repository-file provider

  • We treat the repo file as authoritative user input
  • We install it verbatim
  • If it does not define priority, we do not add or infer one
  • Should accept a priority-key or argument to get user-defined-priority.

repository-url provider

  • Even though we are generating the repo file, the semantics still belong to the user
  • Therefore:
    • If they do not, we generate the repo file with a default priority used for artifact installation

tmt-artifact-shared repository

  • What priority should we give to the tmt-artifact-shared-repository , the repo where we keep the downloaded artifacts? -> We use the default priority set for artifact installation

@thrix thrix removed the status | discuss Needs more discussion before closing label Jan 13, 2026
@thrix thrix removed the status in planning Jan 13, 2026
@thrix thrix moved this to implement in planning Jan 13, 2026
@vaibhavdaren vaibhavdaren force-pushed the vaibhav-augment-priority branch 2 times, most recently from 5a54db7 to fe9ee30 Compare January 13, 2026 15:06
@vaibhavdaren vaibhavdaren moved this from implement to review in planning Jan 13, 2026
Copy link
Copy Markdown
Member

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

Minor suggestion, otherwise LGTM

Comment thread tmt/steps/prepare/artifact/providers/repository.py
@LecrisUT LecrisUT removed their assignment Jan 13, 2026
@thrix
Copy link
Copy Markdown
Contributor

thrix commented Jan 14, 2026

@vaibhavdaren from the description:

The shared artifact repository doesn't take precedence over system repositories (typically priority 99)

I believe this is incorrect, it actually does. default priority is 99, 50 will cause higher prio.

@thrix thrix self-requested a review January 14, 2026 02:42
Introduce DEFAULT_REPOSITORY_PRIORITY (set to 50) to replace the hardcoded
priority value of 1 in create_repository(). This provides a sensible default
for the tmt-artifact-shared repository while allowing user-provided .repo
files to maintain their own priority settings for now
@thrix thrix force-pushed the vaibhav-augment-priority branch from bcde098 to 5cae199 Compare January 14, 2026 08:22
@vaibhavdaren
Copy link
Copy Markdown
Contributor Author

vaibhavdaren commented Jan 14, 2026

@vaibhavdaren from the description:

The shared artifact repository doesn't take precedence over system repositories (typically priority 99)

I believe this is incorrect, it actually does. default priority is 99, 50 will cause higher prio.

@thrix : Fixed the description.

@thrix thrix moved this from review to merge in planning Jan 14, 2026
@thrix thrix merged commit 4319212 into main Jan 14, 2026
25 of 26 checks passed
@thrix thrix deleted the vaibhav-augment-priority branch January 14, 2026 11:30
@github-project-automation github-project-automation Bot moved this from merge to done in planning Jan 14, 2026
@thrix
Copy link
Copy Markdown
Contributor

thrix commented Jan 14, 2026

The failed test is a flake

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

Labels

ci | full test Pull request is ready for the full test execution plugin | artifact Related to the `prepare/artifact` plugin.

Projects

Status: done

Development

Successfully merging this pull request may close these issues.

Set repository priority to a default value for repositories added via repository-url provider

5 participants