Support operating on source repos (#10) #12

Merged

adamwill merged 1 commit from support-src-repos into main 2026-03-17 20:55:03 +00:00

Conversation 5 Commits 1 Files changed 4 +89 -13

adamwill commented 2026-03-17 19:02:55 +00:00

Owner

Copy link

So I happened to find another reason to work on #10 - it's useful
for figuring out the consequences of dropping a package:

quality/tickets#880 (comment)

The good news is, this turns out to be easy. We only need a small
change to the repository modification code to handle source repos,
where the package entries don't specifiy a sourcerpm - we just
take the package name. We identify this case by looking at the
package arch; if it's "src" we just take the name, if it's
anything else we use the existing approach.

With this change, you can just include the appropriate source
repository as a base repository, and otherwise use rmdepcheck
just the same. e.g. this command tests what happens if you drop
the python-pytest-xprocess package from F44:

./rmdepcheck.py --removes https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/x86_64/os/,https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/source/tree/ python-pytest-xprocess

You can see it works because there are results for .src packages
from the source repo; these are build dependencies. If you run the
same command without the source repo you get a smaller set of
results covering only the runtime dependencies.

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

So I happened to find another reason to work on #10 - it's useful for figuring out the consequences of dropping a package: https://forge.fedoraproject.org/quality/tickets/issues/880#issuecomment-579191 The good news is, this turns out to be easy. We only need a small change to the repository modification code to handle source repos, where the package entries don't specifiy a sourcerpm - we just take the package name. We identify this case by looking at the package arch; if it's "src" we just take the name, if it's anything else we use the existing approach. With this change, you can just include the appropriate source repository as a base repository, and otherwise use rmdepcheck just the same. e.g. this command tests what happens if you drop the python-pytest-xprocess package from F44: ./rmdepcheck.py --removes https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/x86_64/os/,https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/source/tree/ python-pytest-xprocess You can see it works because there are results for .src packages from the source repo; these are build dependencies. If you run the same command without the source repo you get a smaller set of results covering only the runtime dependencies. Signed-off-by: Adam Williamson <awilliam@redhat.com>

adamwill added 1 commit 2026-03-17 19:02:55 +00:00

Support operating on source repos (#10) ... e38e57c7ef

So I happened to find another reason to work on #10 - it's useful
for figuring out the consequences of dropping a package:

quality/tickets#880 (comment)

The good news is, this turns out to be easy. We only need a small
change to the repository modification code to handle source repos,
where the package entries don't specifiy a sourcerpm - we just
take the package name. We identify this case by looking at the
package arch; if it's "src" we just take the name, if it's
anything else we use the existing approach.

With this change, you can just include the appropriate source
repository as a base repository, and otherwise use rmdepcheck
just the same. e.g. this command tests what happens if you drop
the python-pytest-xprocess package from F44:

./rmdepcheck.py --removes https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/x86_64/os/,https://dl.fedoraproject.org/pub/fedora/linux/development/44/Everything/source/tree/ python-pytest-xprocess

You can see it works because there are results for .src packages
from the source repo; these are build dependencies. If you run the
same command without the source repo you get a smaller set of
results covering only the runtime dependencies.

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

forgejo-actions commented 2026-03-17 19:04:48 +00:00

Copy link

AI Code Review

📋 MR Summary

Adds support for source repositories by parsing package names directly from the 'name' attribute when the architecture is 'src'.

• Key Changes:
  • Checks the 'common:arch' tag to identify source packages.
  • Extracts source package name directly from 'common:name' for 'src' architectures, bypassing 'rpm:sourcerpm'.
• Impact: rmdepcheck.py
• Risk Level: 🟡 Medium - The logic change introduces a potential AttributeError when parsing non-src packages if the 'sourcerpm' tag is empty or missing, which was previously guarded against.

Detailed Code Review

The implementation correctly identifies source repositories and successfully adjusts how the source package name is parsed. However, a null-check regression was introduced in the else branch for non-source packages. If srpm evaluates to None, calling .rsplit() on it will throw an AttributeError, crashing the script.

📂 File Reviews

📄 `rmdepcheck.py` - Updates primary XML parsing logic to conditionally extract package names based on package architecture.

• Critical [Bug]: Missing null check on srpm before calling .rsplit(). The original code safely checked if srpm and srpm.rsplit..., but the updated code assumes srpm will always be a valid string, which will cause an AttributeError if srpm is None.
  • Suggestion: Add a conditional check or fallback to gracefully handle cases where srpm is None.

        else:
            srpm = mfind(mfind(pkg, "common:format", XMLNS), "rpm:sourcerpm", XMLNS).text
            spkg = srpm.rsplit("-", 2)[0] if srpm else None
        if spkg and spkg in removes:

✅ Summary

• Overall Assessment: One critical issue was identified regarding a missing null check that could lead to runtime exceptions.
• Priority Issues:
  • Address the missing null check for srpm in rmdepcheck.py to prevent AttributeError exceptions.

---

🤖 AI Code Review | Generated with ai-code-review
Platform: Forgejo | AI Provider: gemini | Model: gemini-3-pro-preview

## AI Code Review ### 📋 MR Summary Adds support for source repositories by parsing package names directly from the 'name' attribute when the architecture is 'src'. - **Key Changes:** - Checks the 'common:arch' tag to identify source packages. - Extracts source package name directly from 'common:name' for 'src' architectures, bypassing 'rpm:sourcerpm'. - **Impact:** rmdepcheck.py - **Risk Level:** 🟡 Medium - The logic change introduces a potential AttributeError when parsing non-src packages if the 'sourcerpm' tag is empty or missing, which was previously guarded against. ### Detailed Code Review The implementation correctly identifies source repositories and successfully adjusts how the source package name is parsed. However, a null-check regression was introduced in the `else` branch for non-source packages. If `srpm` evaluates to `None`, calling `.rsplit()` on it will throw an AttributeError, crashing the script. #### 📂 File Reviews <details> <summary><strong>📄 `rmdepcheck.py`</strong> - Updates primary XML parsing logic to conditionally extract package names based on package architecture.</summary> - **Critical** [Bug]: Missing null check on `srpm` before calling `.rsplit()`. The original code safely checked `if srpm and srpm.rsplit...`, but the updated code assumes `srpm` will always be a valid string, which will cause an AttributeError if `srpm` is `None`. - *Suggestion:* Add a conditional check or fallback to gracefully handle cases where `srpm` is None. ```` else: srpm = mfind(mfind(pkg, "common:format", XMLNS), "rpm:sourcerpm", XMLNS).text spkg = srpm.rsplit("-", 2)[0] if srpm else None if spkg and spkg in removes: ```` </details> ### ✅ Summary - **Overall Assessment:** One critical issue was identified regarding a missing null check that could lead to runtime exceptions. - **Priority Issues:** - Address the missing null check for `srpm` in `rmdepcheck.py` to prevent AttributeError exceptions. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) **Platform:** Forgejo | **AI Provider:** gemini | **Model:** gemini-3-pro-preview

adamwill referenced this pull request 2026-03-17 19:06:35 +00:00

Add optional support for testing build dependencies #10

adamwill force-pushed support-src-repos from e38e57c7ef to 79f816ad0e 2026-03-17 19:22:05 +00:00 Compare

adamwill commented 2026-03-17 19:23:50 +00:00

Author

Owner

Copy link

You're quite right. Fixed.

You're quite right. Fixed.

forgejo-actions commented 2026-03-17 19:24:12 +00:00

Copy link

AI Code Review

📋 MR Summary

Adds support for analyzing source repositories by modifying how source packages are identified.

• Key Changes:
  • Updates repository modification logic to handle .src.rpm files.
  • Uses package architecture (arch == "src") to identify source packages and extracts their name instead of relying on sourcerpm.
• Impact: rmdepcheck.py
• Risk Level: 🟢 Low - The changes are localized to metadata parsing logic. The heuristic arch == "src" is an established convention in RPM metadata.

Detailed Code Review

The implementation aligns well with the stated intent of resolving issue #10 by properly identifying .src.rpm files within mixed or dedicated source repositories. The logic uses the agreed-upon heuristic of checking for the src architecture to fall back to the package name directly when the sourcerpm entry might be empty. No critical logic flaws, security vulnerabilities, or performance regressions are identified. The change smoothly integrates with the existing structure. Consider adding minor checks if mfind results are ever None before calling .text to avoid potential AttributeError on malformed repository XML, though this may be handled implicitly by existing mfind functionality or upstream validation.

📂 File Reviews

📄 `rmdepcheck.py` - Modified primary XML parsing to correctly resolve the source package name when the package architecture is 'src'.

• Minor [Bug]: If mfind returns None for missing elements in malformed XML, calling .text directly on the return value will raise an AttributeError.
  • Suggestion: Add safe-guards or verify that mfind guarantees returning an element or handles None transparently.

arch_element = mfind(pkg, "common:arch", XMLNS)
if arch_element is not None and arch_element.text == "src":
    spkg_element = mfind(pkg, "common:name", XMLNS)
    spkg = spkg_element.text if spkg_element is not None else None

✅ Summary

• Overall Assessment: Review completed. No critical issues found. The implementation successfully expands the tool's capabilities with low risk.
• Minor Suggestions:
  • Ensure mfind calls handle None values gracefully to prevent AttributeError on malformed repository metadata.

---

🤖 AI Code Review | Generated with ai-code-review
Platform: Forgejo | AI Provider: gemini | Model: gemini-3-pro-preview

## AI Code Review ### 📋 MR Summary Adds support for analyzing source repositories by modifying how source packages are identified. - **Key Changes:** - Updates repository modification logic to handle `.src.rpm` files. - Uses package architecture (`arch == "src"`) to identify source packages and extracts their name instead of relying on `sourcerpm`. - **Impact:** rmdepcheck.py - **Risk Level:** 🟢 Low - The changes are localized to metadata parsing logic. The heuristic `arch == "src"` is an established convention in RPM metadata. ### Detailed Code Review The implementation aligns well with the stated intent of resolving issue #10 by properly identifying `.src.rpm` files within mixed or dedicated source repositories. The logic uses the agreed-upon heuristic of checking for the `src` architecture to fall back to the package name directly when the `sourcerpm` entry might be empty. No critical logic flaws, security vulnerabilities, or performance regressions are identified. The change smoothly integrates with the existing structure. Consider adding minor checks if `mfind` results are ever `None` before calling `.text` to avoid potential `AttributeError` on malformed repository XML, though this may be handled implicitly by existing `mfind` functionality or upstream validation. #### 📂 File Reviews <details> <summary><strong>📄 `rmdepcheck.py`</strong> - Modified primary XML parsing to correctly resolve the source package name when the package architecture is 'src'.</summary> - **Minor** [Bug]: If `mfind` returns `None` for missing elements in malformed XML, calling `.text` directly on the return value will raise an `AttributeError`. - *Suggestion:* Add safe-guards or verify that `mfind` guarantees returning an element or handles `None` transparently. ```` arch_element = mfind(pkg, "common:arch", XMLNS) if arch_element is not None and arch_element.text == "src": spkg_element = mfind(pkg, "common:name", XMLNS) spkg = spkg_element.text if spkg_element is not None else None ```` </details> ### ✅ Summary - **Overall Assessment:** Review completed. No critical issues found. The implementation successfully expands the tool's capabilities with low risk. - **Minor Suggestions:** - Ensure `mfind` calls handle `None` values gracefully to prevent `AttributeError` on malformed repository metadata. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) **Platform:** Forgejo | **AI Provider:** gemini | **Model:** gemini-3-pro-preview

adamwill force-pushed support-src-repos from 79f816ad0e to b26936900d 2026-03-17 19:34:45 +00:00 Compare

adamwill commented 2026-03-17 19:35:14 +00:00

Author

Owner

Copy link

Ensure mfind calls handle None values gracefully to prevent AttributeError on malformed repository metadata.

Well, it doesn't exactly handle them "gracefully", it raises an exception if it hits None. But this is intentional, I want an exception to happen in that case.

`Ensure mfind calls handle None values gracefully to prevent AttributeError on malformed repository metadata.` Well, it doesn't exactly handle them "gracefully", it raises an exception if it hits None. But this is intentional, I want an exception to happen in that case.

adamwill force-pushed support-src-repos from b26936900d to c212769e1b 2026-03-17 20:44:53 +00:00 Compare

adamwill force-pushed support-src-repos from c212769e1b to cd1d5aa100 2026-03-17 20:46:54 +00:00 Compare

adamwill force-pushed support-src-repos from cd1d5aa100 to a116bc02ff 2026-03-17 20:49:13 +00:00 Compare

forgejo-actions commented 2026-03-17 20:52:02 +00:00

Copy link

AI Code Review

📋 MR Summary

This MR adds support for parsing source repositories by identifying .src packages and extracting their names directly instead of relying on the sourcerpm attribute.

• Key Changes:
  • Updated replace_primary in rmdepcheck.py to branch package processing based on the package architecture (arch == "src").
  • Preserved standard processing for binary packages, ensuring backwards compatibility.
  • Parameterized tests in test_rmdepcheck.py to validate both binary and source repository metadata structures.
• Impact: rmdepcheck.py, tests/test_rmdepcheck.py
• Risk Level: 🟢 Low - The changes introduce standard branching logic to handle source packages without disrupting the existing workflow for binary packages. The author has clarified that exceptions resulting from malformed metadata are intentional.

Detailed Code Review

The implementation correctly aligns with the stated goal of supporting source repositories to identify build dependencies. The logic efficiently distinguishes between binary and source packages by reading the arch tag and extracts the appropriate identifier for exclusion checks. The accompanying parameterization of the test suite comprehensively covers the new workflow without complicating the test codebase.

Since the author has previously confirmed that hard failures (e.g., AttributeError from missing metadata tags) are the intended behavior for malformed repositories, the lack of explicit None checks on the mfind return values is considered correct for this project's context.

✅ Summary

• Overall Assessment: The changes are solid, well-tested, and introduce no apparent regressions or logic errors. The PR is ready to merge.

---

🤖 AI Code Review | Generated with ai-code-review
Platform: Forgejo | AI Provider: gemini | Model: gemini-3-pro-preview

## AI Code Review ### 📋 MR Summary This MR adds support for parsing source repositories by identifying `.src` packages and extracting their names directly instead of relying on the `sourcerpm` attribute. - **Key Changes:** - Updated `replace_primary` in `rmdepcheck.py` to branch package processing based on the package architecture (`arch == "src"`). - Preserved standard processing for binary packages, ensuring backwards compatibility. - Parameterized tests in `test_rmdepcheck.py` to validate both binary and source repository metadata structures. - **Impact:** rmdepcheck.py, tests/test_rmdepcheck.py - **Risk Level:** 🟢 Low - The changes introduce standard branching logic to handle source packages without disrupting the existing workflow for binary packages. The author has clarified that exceptions resulting from malformed metadata are intentional. ### Detailed Code Review The implementation correctly aligns with the stated goal of supporting source repositories to identify build dependencies. The logic efficiently distinguishes between binary and source packages by reading the `arch` tag and extracts the appropriate identifier for exclusion checks. The accompanying parameterization of the test suite comprehensively covers the new workflow without complicating the test codebase. Since the author has previously confirmed that hard failures (e.g., `AttributeError` from missing metadata tags) are the intended behavior for malformed repositories, the lack of explicit `None` checks on the `mfind` return values is considered correct for this project's context. ### ✅ Summary - **Overall Assessment:** The changes are solid, well-tested, and introduce no apparent regressions or logic errors. The PR is ready to merge. --- 🤖 **AI Code Review** | Generated with [ai-code-review](https://gitlab.com/redhat/edge/ci-cd/ai-code-review) **Platform:** Forgejo | **AI Provider:** gemini | **Model:** gemini-3-pro-preview

adamwill merged commit a116bc02ff into main 2026-03-17 20:55:03 +00:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

  

ai-review-please

Request AI review for a pull request

  

Backlog Status

Needs Review

Work items that have been created or updated but need review, refinement, or approval before they can be considered ready for sprint planning.

  

Backlog Status

Ready

Work items that have been reviewed, refined, and are ready to be committed to a sprint. These items have clear requirements and can be worked on immediately when capacity is available.

  

chore

Work that needs to be done but doesn't add new functionality or fix bugs. Maintenance tasks that keep the system running smoothly and the codebase clean.

  

documentation

Work focused on creating, updating, or improving documentation for users, developers, or system administrators.

  

points

01

A task that is simple, small, and has very little uncertainty. It should be easy enough for any team member to pick up and complete quickly, without a lot of discussion or research.

  

points

02

This is a task that is slightly more complex or larger than a "1." It might involve two or three simple steps, but the outcome is still very clear and there's little uncertainty.

  

points

03

Medium-sized task with some noticeable complexity. It might require a bit more thought or collaboration. The steps are well-defined, but it's not a trivial change.

  

points

05

A significant piece of work. It has more complexity and a higher degree of uncertainty than smaller tasks. It might require some research, a design discussion, or collaboration across multiple teams.

  

points

08

Complex task with a lot of uncertainty. It's often the largest size that a team will consider for a single sprint. It likely requires a detailed breakdown and multiple team members to complete.

  

points

13

Any task that feels like it's a "13" or more is considered an Epic. These are too large and uncertain to be worked on in a single sprint and should be broken down.

  

pr2jira

Sync this pull request to Jira for Agility tracking. Only use for PRs without a corresponding ticket.

  

Priority

Critical

This is for work that is urgent and of critical importance and so and must be addressed immediately with a higher priority than all other work, even if not originally planned for the current sprint.

  

Priority

High

This is for work that is critical and needs to be addressed in the next sprint. It's often for urgent bugs, important features, or a major piece of technical debt that is blocking other work.

  

Priority

Low

This is for work that is good to do but not urgent. It's a candidate for being worked on when there's extra time or when it rises in importance. It can stay in the backlog for a longer period.

  

Priority

Medium

This is for important work that should be done soon, but isn't as urgent as a high-priority item. It will likely be considered for the next 1-2 sprints.

  

Sprint Status

Blocked

Work items that cannot progress due to external dependencies, blockers, or issues that need resolution.

  

Sprint Status

Done

Work items that have been completed, reviewed, and integrated. No further work is needed within the current sprint.

  

Sprint Status

In Progress

Work items currently being worked on by a team member. Active development, research, or implementation is happening.

  

Sprint Status

Review

Work items that are complete but need review, testing, or approval before being marked as done.

  

Sprint Status

To Do

Work items that are committed to the sprint but haven't been started yet. These items are ready to be picked up by team members.

  

Technical Debt

Work that addresses shortcuts, workarounds, or suboptimal implementations that were done previously to meet deadlines. These items improve code quality, maintainability, or performance without adding new features.

  

Work Item

Bug

Issues where the system is not working as intended or designed. Bugs represent deviations from expected behavior.

  

Work Item

Epic

Large initiatives that are broken down into multiple smaller work items (user stories, tasks, bugs). Epics represent significant features or projects that span multiple sprints.

  

Work Item

Spike

Time-boxed research, investigation, or proof-of-concept work to reduce uncertainty or explore solutions. Spikes help inform future development decisions.

  

Work Item

Task

Used to break down larger user stories into smaller, manageable implementation pieces. They focus on the specific technical steps needed to deliver the user story's value.

  

Work Item

User Story

User-facing features or functionality that can be framed from a user perspective, describing what someone wants to accomplish and why.

No labels

ai-review-please

Backlog Status

Needs Review

Backlog Status

Ready

chore

documentation

points

01

points

02

points

03

points

05

points

08

points

13

pr2jira

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Sprint Status

Blocked

Sprint Status

Done

Sprint Status

In Progress

Sprint Status

Review

Sprint Status

To Do

Technical Debt

Work Item

Bug

Work Item

Epic

Work Item

Spike

Work Item

Task

Work Item

User Story

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

quality/rmdepcheck!12

No description provided.