Skip to content

Conversation

@kpcyrd
Copy link
Owner

@kpcyrd kpcyrd commented Nov 17, 2025

This should fix #206.

As can be seen in the unit test, I'm slightly worried this may add unexpected empty groups, flagging all packages as removed.

cc: @jspricke

@kpcyrd kpcyrd requested a review from Copilot November 18, 2025 09:16
Copilot finished reviewing on behalf of kpcyrd November 18, 2025 09:17
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

This PR fixes issue #206 by adding support for syncing Debian releases that have no packages. The change ensures that empty release groups are created even when there are no packages to import, preventing unexpected behavior when a release/component/architecture combination exists but contains no packages.

Key changes:

  • Refactored get_mut_group to extract release group creation logic into a new create_release_group method
  • Added pre-creation of release groups for all architectures before importing packages in both compressed and uncompressed binary package import functions
  • Added test coverage for empty releases and updated existing test to expect empty architecture groups

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

Comment on lines 409 to 411
for arch in &sync.architectures {
self.create_release_group(release, component, arch);
}
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

The loop creates release groups for all architectures from sync.architectures, but this may create empty groups for architectures that don't match the packages being imported. Consider checking if the packages in the file match these architectures to avoid creating spurious empty groups. However, based on the test case at line 1851-1860, this appears to be intentional behavior to ensure all configured architectures have groups even when empty. If this is the desired behavior, consider adding a comment explaining why all architectures need groups created upfront.

Copilot uses AI. Check for mistakes.
Comment on lines 426 to 428
for arch in &sync.architectures {
self.create_release_group(release, component, arch);
}
Copy link

Copilot AI Nov 18, 2025

Choose a reason for hiding this comment

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

This code duplicates the logic from import_compressed_binary_package_file (lines 409-411). Consider extracting this into a helper method or documenting why this duplication is necessary for both compressed and uncompressed imports.

Copilot uses AI. Check for mistakes.
@jspricke
Copy link
Contributor

Thanks for working on this!

As can be seen in the unit test, I'm slightly worried this may add unexpected empty groups, flagging all packages as removed.

You could check the signature of the repo to verify that is correct. Also rebuilderd no longer deletes results so I don't think there is much to worry.

According to @h01ger this PR is running on reproduce.d.n but I don't think it is working:

https://reproduce.debian.net/all/trixie-proposed-updates.html

@h01ger
Copy link
Contributor

h01ger commented Nov 19, 2025

you misunderstood me. I have merged this in my branch, but not deployed it, because it doesn't build (on trixie, havent tried elsewhere)

@jspricke
Copy link
Contributor

Works:

Nov 19 15:14:09 osuosl5-amd64 rebuilderd_pkgsync.sh[237086]: [2025-11-19T15:14:09Z INFO  rebuildctl] Sending debian/trixie-proposed-updates/main (all) to rebuilderd (0 packages)...

https://reproduce.debian.net/all/trixie-proposed-updates.html

@kpcyrd kpcyrd merged commit eccfd07 into main Nov 19, 2025
13 checks passed
@kpcyrd kpcyrd deleted the empty-groups branch November 19, 2025 16:26
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.

Empty package list is not synced to the rebuilderd

4 participants