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

Use new RPMTAG_MODULARITYLABEL to identify modular RPMs #1368

Merged
merged 1 commit into from
Jun 12, 2019

Conversation

goosemania
Copy link
Member

Old RPMTAG_DISTTAG is still in the code for the old content
that might be still around.

closes #4869
https://pulp.plan.io/issues/4869

Old RPMTAG_DISTTAG is still in the code for the old content
that might be still around.

closes #4869
https://pulp.plan.io/issues/4869

# Id of the tag which is used to analyze RPM headers at upload or download time.
# It's availiable in rpm package in F30+ and Centos8+.
RPMTAG_MODULARITYLABEL = 5096

This comment was marked as outdated.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind

@dralley
Copy link
Contributor

dralley commented Jun 10, 2019

This looks correct to me. Can I easily test this?

@goosemania
Copy link
Member Author

Upload a modular rpm into Pulp and look at this content, its is_modular attribute should be set to True

@bherrin3
Copy link

bherrin3 commented Jun 11, 2019

@dralley @goosemania I will be using the following fixture location. Mind seeing if the modular RPM I stole from Fedora works?

This will later be updated to a generated fixture, but will suffice for initial verification.

If this RPM is not is_modular, please provide one so I have a sane test baseline.

Thanks!

Edit: https://repos.fedorapeople.org/pulp/pulp/fixtures/rpm-with-modular/

@bherrin3
Copy link

bherrin3 commented Jun 11, 2019

@dralley @goosemania The PR for this test in Pulp-2-Tests is available to use.

If that is used and verified good, we can knock out two PR-birds with one PR-verification.

I would appreciate knowing it if works with the new updates as well ;)

PR Branch with Test

pulp/Pulp-2-Tests#201

@dralley
Copy link
Contributor

dralley commented Jun 11, 2019

@goosemania @pulp/qe I tested this manually and it works for me, but unfortunately I destroyed by vagrant box literally seconds before I saw this comment.

@dralley
Copy link
Contributor

dralley commented Jun 12, 2019

@goosemania I ran the QE provided test for uploading an RPM and it failed. The sync test passed, but it passed prior to applying the patch also. It uses a different RPM.

The RPM comes from here: https://repos.fedorapeople.org/repos/pulp/pulp/fixtures/rpm-with-modular/

I looked at the RPM headers with this command I got from the internet.

rpm -qp nodejs-10.15.2-1.module_f30+3181+3be24b3a.x86_64.rpm --qf "$(rpm --querytags | sed -nr 's/(.*)/\1:%{\1}/p' | tr '[:space:]' '\n')"

and it gave

MODULARITYLABEL:nodejs:10:3020190301191749:a5b0195c

so the tag is definitely there but maybe it doesn't match the regex

@goosemania
Copy link
Member Author

goosemania commented Jun 12, 2019

@dralley, @bherrin3 , if modularitylabel is set, there is no check for regex, regex is for old tag.
I'll take a look later today.

@goosemania
Copy link
Member Author

@dralley , @bherrin3, I re-tested manually and I ran the test provided in pulp/Pulp-2-Tests#201 - it succeeded.

Copy link
Contributor

@dralley dralley left a comment

Choose a reason for hiding this comment

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

Confirmed that the failure was an environment problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants