quality/rmdepcheck

Fork 1

Add non-checked base repo concept (#37) #38

Closed

adamwill wants to merge 1 commit from nonchecked-base into main

adamwill commented

2026-04-29 22:14:51 +00:00

Owner

This adds a 'non-checked base repo' concept to address the ELN
scenario covered in #37. A 'non-checked base repo' is just like
a base repo, but it is not included in the --check arg when
generating the modified repoclosure. So it's always available to
the depsolver, and it is modified just like a regular base repo
(unlike a 'non-modified base repo'), but we don't actually check
for repoclosure issues in it.

This is intended to be used for the ELN buildroot repo. We need
it to be in scope for the depsolver so we do not get spurious
failures when a package under testing adds a dependency on
something that is only in the buildroot (this is fine if that
package is itself a buildroot-only package). But @yselkowitz
does not want us to fail when an update would introduce a new
dependency issue in a buildroot-only package, so we do not want
to run repoclosure on the buildroot repo.

Signed-off-by: Adam Williamson awilliam@redhat.com

This adds a 'non-checked base repo' concept to address the ELN scenario covered in #37. A 'non-checked base repo' is just like a base repo, but it is not included in the `--check` arg when generating the modified repoclosure. So it's always available to the depsolver, and it is modified just like a regular base repo (unlike a 'non-modified base repo'), but we don't actually check for repoclosure issues in it. This is intended to be used for the ELN buildroot repo. We need it to be in scope for the depsolver so we do not get spurious failures when a package under testing adds a dependency on something that is only in the buildroot (this is fine if that package is itself a buildroot-only package). But @yselkowitz does not want us to fail when an update would *introduce* a new dependency issue in a buildroot-only package, so we do not want to run repoclosure on the buildroot repo. Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill added 1 commit

2026-04-29 22:14:51 +00:00

Add non-checked base repo concept (#37 )

CI via Tox / tox (pull_request) Successful in 1m29s

Details

AI Code Review / ai-review (pull_request_target) Successful in 15s

Details

c53ac8ac71

This adds a 'non-checked base repo' concept to address the ELN
scenario covered in #37. A 'non-checked base repo' is just like
a base repo, but it is not included in the `--check` arg when
generating the modified repoclosure. So it's always available to
the depsolver, and it is modified just like a regular base repo
(unlike a 'non-modified base repo'), but we don't actually check
for repoclosure issues in it.

This is intended to be used for the ELN buildroot repo. We need
it to be in scope for the depsolver so we do not get spurious
failures when a package under testing adds a dependency on
something that is only in the buildroot (this is fine if that
package is itself a buildroot-only package). But @yselkowitz
does not want us to fail when an update would *introduce* a new
dependency issue in a buildroot-only package, so we do not want
to run repoclosure on the buildroot repo.

Signed-off-by: Adam Williamson <awilliam@redhat.com>

forgejo-actions commented

2026-04-29 23:06:59 +00:00

AI Code Review

📋 MR Summary

Adds a 'non-checked base repo' concept, allowing certain repositories to be modified and used for dependency resolution without being subjected to repoclosure checks.

Key Changes:
- Added --ncbaserepos argument for non-checked base repositories.
- Updated get_base_repoclosure to include non-checked repos in the solver while excluding them from the --check argument.
- Modified get_modified_and_new_repoclosure to apply package exclusions (modifications) to non-checked base repos just like regular base repos.
- Added end-to-end tests (test_e2e_eln) and mock repositories to verify the ELN buildroot scenario.
Impact: rmdepcheck.py, tests/test_rmdepcheck.py, tests/testdata
Risk Level: 🟢 Low - The changes add optional parameters that do not disrupt the existing workflow unless explicitly invoked. The logic for including and excluding repos from the check phase is cleanly implemented.

Detailed Code Review

The implementation correctly aligns with the stated intent of supporting ELN buildroot scenarios. The ncbaserepos are appropriately included in the dependency solver and modified via exclusions, while being safely omitted from the --check list in both get_base_repoclosure and get_modified_and_new_repoclosure.

The test coverage has been nicely updated with an end-to-end test that properly validates the expected behavior (ignoring broken deps in the buildroot repo while catching them in the shipped repo). No security or performance issues were identified.

✅ Summary

Overall Assessment: The changes are well-implemented and adequately tested. No critical issues were identified during the review.
Minor Suggestions:
- Consider adding type hints to tests/testdata/repos/mkrepos.py to maintain consistency with the main codebase.

🤖 AI Code Review | Generated with ai-code-review | Model: gemini-3.1-pro-preview

⚠️ AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.

## AI Code Review ### 📋 MR Summary Adds a 'non-checked base repo' concept, allowing certain repositories to be modified and used for dependency resolution without being subjected to repoclosure checks. - **Key Changes:** - Added `--ncbaserepos` argument for non-checked base repositories. - Updated `get_base_repoclosure` to include non-checked repos in the solver while excluding them from the `--check` argument. - Modified `get_modified_and_new_repoclosure` to apply package exclusions (modifications) to non-checked base repos just like regular base repos. - Added end-to-end tests (`test_e2e_eln`) and mock repositories to verify the ELN buildroot scenario. - **Impact:** rmdepcheck.py, tests/test_rmdepcheck.py, tests/testdata - **Risk Level:** 🟢 Low - The changes add optional parameters that do not disrupt the existing workflow unless explicitly invoked. The logic for including and excluding repos from the check phase is cleanly implemented. ### Detailed Code Review The implementation correctly aligns with the stated intent of supporting ELN buildroot scenarios. The `ncbaserepos` are appropriately included in the dependency solver and modified via exclusions, while being safely omitted from the `--check` list in both `get_base_repoclosure` and `get_modified_and_new_repoclosure`. The test coverage has been nicely updated with an end-to-end test that properly validates the expected behavior (ignoring broken deps in the buildroot repo while catching them in the shipped repo). No security or performance issues were identified. ### ✅ Summary - **Overall Assessment:** The changes are well-implemented and adequately tested. No critical issues were identified during the review. - **Minor Suggestions:** - Consider adding type hints to `tests/testdata/repos/mkrepos.py` to maintain consistency with the main codebase. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) | **Model:** `gemini-3.1-pro-preview` ⚠️ *AI-generated suggestions may be incorrect. Verify before applying. Not a replacement for human review.*

adamwill referenced this pull request

2026-04-29 23:13:14 +00:00

Allow skipping 'basetest' or 'newtest' phases (#37) #39

adamwill commented

2026-04-29 23:14:00 +00:00

Author

Owner

This is an alternative to #39. See #39 (comment) for my thoughts on how they compare.

This is an alternative to #39. See https://forge.fedoraproject.org/quality/rmdepcheck/pulls/39#issuecomment-677170 for my thoughts on how they compare.

adamwill referenced this pull request from ci/tickets

2026-04-29 23:20:14 +00:00

ELN want rmdepcheck to run against composed repos, not buildroot #555

adamwill referenced this pull request from quality/paperwork

2026-04-30 07:00:30 +00:00

openQA failure monitoring 2026 sprint 8 #37

adamwill commented

2026-05-02 07:24:47 +00:00

Author

Owner

Rolled into #40 .

adamwill closed this pull request