Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix repo metadata information #49

Merged
merged 1 commit into from
Aug 28, 2018

Conversation

rabadin
Copy link

@rabadin rabadin commented Aug 14, 2018

Fix the 'Label' and the 'Description' in the repo metadata (Release file).
This enables Apt Pinning because pinning uses this metadata to distinguish between repos.

Fixes #3917
https://pulp.plan.io/issues/3917

@pulpbot
Copy link
Member

pulpbot commented Aug 14, 2018

Can one of the admins verify this patch?

1 similar comment
@pulpbot
Copy link
Member

pulpbot commented Aug 14, 2018

Can one of the admins verify this patch?

Copy link
Member

@mibanescu mibanescu 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 for the fixes!

In order to merge this, I need two things:

  • please rebase the patch to the latest HEAD on master
  • I want to make sure we're capturing this in the test suite.

While fixing a test I noticed that the default component doesn't get its Label and Description set, so I had to fix that too. Here is the patch you will have to apply, and force-push to your branch:

diff --git a/plugins/pulp_deb/plugins/distributors/distributor.py b/plugins/pulp_deb/plugins/distributors/distributor.py
index 8a83b2a..cdcf9b7 100644
--- a/plugins/pulp_deb/plugins/distributors/distributor.py
+++ b/plugins/pulp_deb/plugins/distributors/distributor.py
@@ -285,6 +285,7 @@ class MetadataStep(PluginStep):
 
         sign_options = configuration.get_gpg_sign_options(self.get_repo(),
                                                           self.get_config())
+        repo = self.get_repo()
 
         for release_unit in release_units:
             codename = release_unit.codename
@@ -309,7 +310,6 @@ class MetadataStep(PluginStep):
                 arch_units['all'] = all_units
                 comp_arch_units[component_unit.name] = arch_units
 
-            repo = self.get_repo()
             repometa = aptrepo.AptRepoMeta(
                 codename=codename,
                 components=[comp.name for comp in rel_components],
@@ -366,6 +366,8 @@ class MetadataStep(PluginStep):
                     codename=codename,
                     components=[component_name],
                     architectures=list(architectures),
+                    description=repo.description,
+                    label=repo.id,
                 )
                 arepo = aptrepo.AptRepo(self.get_working_dir(),
                                         repo_name=self.get_repo().id,
diff --git a/plugins/test/unit/plugins/distributors/test_distributor.py b/plugins/test/unit/plugins/distributors/test_distributor.py
index b556bd3..048055b 100644
--- a/plugins/test/unit/plugins/distributors/test_distributor.py
+++ b/plugins/test/unit/plugins/distributors/test_distributor.py
@@ -4,6 +4,7 @@ import sys
 import time
 import uuid
 import hashlib
+from debian import deb822
 
 import mock
 from .... import testbase
@@ -160,6 +161,7 @@ class PublishRepoMixIn(object):
         repo.configure_mock(
             working_dir=os.path.join(self.work_dir, 'work_dir'),
             content_unit_counts=unit_counts,
+            description="description here",
             id=repo_id)
 
         def mock_get_units(repo_id, model_class, *args, **kwargs):
@@ -249,6 +251,10 @@ class PublishRepoMixIn(object):
                 for arch in self.Architectures:
                     self.assertTrue(os.path.exists(
                         os.path.join(comp_dir, comp, 'binary-' + arch, 'Packages')))
+            # #3917: make sure Description and Label are properly set
+            rel_file_contents = deb822.Deb822(sequence=open(release_file))
+            self.assertEqual(repo.id, rel_file_contents['Label'])
+            self.assertEqual(repo.description, rel_file_contents['Description'])
 
         exp = [
             mock.call(repo.id, models.DebRelease, None),

@rabadin rabadin force-pushed the fix-label-description branch 3 times, most recently from c8895b3 to abab699 Compare August 28, 2018 08:57
Fix the 'Label' and the 'Description' in the repo metadata
('Release' file).
This enables Apt Pinning because pinning uses this metadata to
distinguish between repos.

Fixes #3917
https://pulp.plan.io/issues/3917
@rabadin
Copy link
Author

rabadin commented Aug 28, 2018

Thanks @mibanescu, I've rebased onto current master and integrated your patch (with a minor change to the import part and the test).

@mibanescu mibanescu merged commit 92d79cf into pulp:master Aug 28, 2018
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.

3 participants