Fix naming of packages added to target koji tag #105
Conversation
rpmlb/builder/koji.py
Outdated
| collection=self.collection, | ||
| name=package_dict['name'], | ||
| )) | ||
| if package_dict['name'] == self.collection: # metapackage |
There was a problem hiding this comment.
Make this a function. You can then test the function isolated without all the bloody mocking.
There was a problem hiding this comment.
Yeah, bit of a brainf*rt on my part. I will take time to do this properly next week.
Thanks for keeping the technical debt at bay ☺ !
khardix
force-pushed
the
fix/add-package-naming
branch
2 times, most recently
from
de3ed20
to
22854c6
Compare
|
Next attempt at this fix. This time, I left it in two commits:
|
rpmlb/builder/koji.py
Outdated
| """ | ||
|
|
||
| # The metapackage, or the base_name already has the prefix applied. | ||
| if base_name.startswith(collection): |
There was a problem hiding this comment.
Should it not start with colection+'-'?
There was a problem hiding this comment.
No, because the name of the metapackage is exactly collection. base_name is not NVR, just the "N" part.
There was a problem hiding this comment.
So in fact it should be: if base_name.startswith(collection + '-') or base_name == collection:?
There was a problem hiding this comment.
Basically, yes. The important part is the base_name == collection. The startswith check is best-effort to avoid duplicate attachment of the prefix if the name already starts with it (and so in fact it is full_name).
If we are afraid of false positives here, I would drop the startswith check entirely.
There was a problem hiding this comment.
I'm just being paranoid. However I was bit by almost the same thing in the past. See pytest-dev/pytest#2939 (comment)
There was a problem hiding this comment.
I understand. The trouble here is that I'm not sure if the '-' at the end would help anyhow 😉.
Since the problem I'm trying to solve here manifests (so far) only on metapackages, I will drop the startswith check, which should make the condition unambiguous. It can be always added back if we run into a case that needs it.
- Turn static methods for rebuild steps into regular ones - Extract mocking in tests into dedicated fixture
khardix
force-pushed
the
fix/add-package-naming
branch
from
22854c6
to
27364ad
Compare
KojiBuilderwas incorrectly handling naming of metapackages when adding them to target tag, essentialy duplicated the collection name (i.e. addingrh-ror50-rh-ror50in place of justrh-ror50).This fix adds a check for the metapackage name, avoiding the duplication.