[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

ceph.spec.in: python-argparse only in Python 2.6 #4970

Merged
merged 1 commit into from
Jun 26, 2015
Merged

ceph.spec.in: python-argparse only in Python 2.6 #4970

merged 1 commit into from
Jun 26, 2015

Conversation

smithfarm
Copy link
Contributor 
ceph.spec.in: python-argparse only in Python 2.6

argparse is a widely-used Python module for parsing command-line arguments.
Ceph makes heavy use of Python scripts, both in the build environment and on
cluster nodes and clients.

Until Python 2.6, argparse was distributed separately from Python proper.
As of 2.7 it is part of the Python standard library.

Although the python package in a given distro may or may not Provide:
python-argparse, this cannot be relied upon.

Therefore, this commit puts appropriate conditionals around Requires:
python-argparse and BuildRequires: python-argparse. It does so for Red
Hat/CentOS and SUSE only, because the last Fedora version with Python 2.6
was Fedora 13, which is EOL.

http://tracker.ceph.com/issues/12034 Fixes: #12034

Signed-off-by: Nathan Cutler <ncutler@suse.com>

@alfredodeza
Copy link
Contributor

This looks good. But, to prevent this from happening we should start checking for the compatibility problems. This is defined in http://tracker.ceph.com/issues/8458

@smithfarm
Copy link
Contributor Author

@alfredodeza So I should make a tox.ini file, etc. for this case based on your #1879 to prepare for getting it into make check ?

@alfredodeza
Copy link
Contributor

@smithfarm I think that work with the tox.ini is completed. However I am not sure what would be the right way to implement it as part of make check.

The tox.ini for ceph-disk is here: https://github.com/ceph/ceph/blob/master/src/test/python/ceph-disk/tox.ini

And it does work correctly.

@smithfarm
Copy link
Contributor Author

@alfredodeza Would you be willing to implement a tox.ini, setup.py etc. for the ceph script? This PR is about not being able to run ceph when ceph-common is installed all by itself in a Python 2.6 environment.

@alfredodeza
Copy link
Contributor

Looks good to me

Reviewed-by: Alfredo Deza adeza@redhat.com

@loic-bot
Copy link

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/5911/

@smithfarm
Copy link
Contributor Author

Thanks to @tchaikov, the gitbuilders are now crunching wip-4970

@ktdreyer
Copy link
Member

The basic idea looks good to me. Thanks for fixing this.

At the moment, RHEL 7's python package provides the python-argparse package, but Fedora's python3 package has dropped this bit of backwards-compatibility. I think we should place some dist tag conditionals around this so it doesn't break new future distros. So to satisfy RHEL 6/CentOS 6, we can specify:

%if 0%{?rhel} <= 6
Requires: python-argparse
%endif

@smithfarm , what would the appropriate SLES conditional be there? I'm sorry I don't know specifically which SUSE version ships Python 2.6.

@smithfarm
Copy link
Contributor Author

All of the more recent SLE and openSUSE boxes I could find were running Python 2.7. Based on https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto I'm guessing an appropriate conditional would be

%if 0%{?suse_version} < 1110

but I will try to find out with more certainty.

@ktdreyer
Copy link
Member

ok cool, thanks @smithfarm

@smithfarm smithfarm changed the title ceph.spec.in: ceph-common needs python-argparse unconditionally ceph.spec.in: python-argparse only in Python 2.6 Jun 25, 2015
@loic-bot
Copy link

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6107/

@ktdreyer
Copy link
Member

I think we can simplify the Requires: python-argparse further. ceph requires ceph-common, and ceph-common requires python-argparse.

Since this is so complicated, we don't need to copy and paste the whole Requires bits for the main ceph package as well.

That will also have the nice side effect of eliminating the duplicate BuildRequires.

@smithfarm
ceph.spec.in: python-argparse only in Python 2.6
23171c9 
argparse is a widely-used Python module for parsing command-line arguments.
Ceph makes heavy use of Python scripts, both in the build environment and on
cluster nodes and clients.

Until Python 2.6, argparse was distributed separately from Python proper.
As of 2.7 it is part of the Python standard library.

Although the python package in a given distro may or may not Provide:
python-argparse, this cannot be relied upon.

Therefore, this commit puts appropriate conditionals around Requires:
python-argparse and BuildRequires: python-argparse. It does so for Red
Hat/CentOS and SUSE only, because the last Fedora version with Python 2.6
was Fedora 13, which is EOL.

argparse is required by both the ceph and ceph-common packages, but since ceph
requires ceph-common, the argparse Requires and BuildRequires need only appear
once, under ceph-common.

http://tracker.ceph.com/issues/12034 Fixes: ceph#12034

Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

@ktdreyer That certainly makes it a lot simpler!

@loic-bot
Copy link

SUCCESS: http://jenkins.ceph.dachary.org/job/ceph/6128/

@ktdreyer
Copy link
Member

Looks great! merging

@ktdreyer ktdreyer merged commit 23171c9 into ceph:master Jun 26, 2015
ktdreyer added a commit that referenced this pull request Jun 26, 2015
@ktdreyer
Merge pull request #4970 from SUSE/wip-12034-master
e579e12 
ceph.spec.in: python-argparse only in Python 2.6

Reviewed-by: Alfredo Deza <adeza@redhat.com>
Reviewed-by: Ken Dreyer <kdreyer@redhat.com>
@smithfarm smithfarm deleted the wip-12034-master branch June 26, 2015 17:49
@smithfarm
Copy link
Contributor Author

@ktdreyer Thanks! Can you delete the wip-4970 branch?

@ktdreyer
Copy link
Member

Sure!

$ git push upstream :wip-4970
To git@github.com:ceph/ceph.git
 - [deleted]         wip-4970

@alfredodeza alfredodeza mentioned this pull request Jul 1, 2015
Merged
@smithfarm smithfarm mentioned this pull request Jul 6, 2015
Merged
@smithfarm
Copy link
Contributor Author

@dachary I marked the original issue "Pending Backport" and created the corresponding backport tracker issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ktdreyer ktdreyer

Labels
bug-fix build/ops
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@smithfarm @alfredodeza @loic-bot @ktdreyer