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

build/ops: rpm: Python 3-only ceph-disk and ceph-volume #20140

Merged
merged 2 commits into from Feb 2, 2018
Merged

build/ops: rpm: Python 3-only ceph-disk and ceph-volume #20140

merged 2 commits into from Feb 2, 2018

Conversation

smithfarm
Copy link
Contributor

@smithfarm smithfarm commented Jan 27, 2018 •
edited

Follow-up PR to #20018

@smithfarm smithfarm changed the title build/ops: rpm: py3-only ceph-disk and ceph-volume build/ops: rpm: Python 3-only ceph-disk and ceph-volume Jan 27, 2018
tchaikov
tchaikov reviewed Jan 27, 2018
View reviewed changes
ceph.spec.in
%{python_sitelib}/ceph_detect_init*
%{python_sitelib}/ceph_disk*
%else
%{python3_sitelib}/ceph_detect_init*
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to use %{python%{_python_buildid}_sitelib} ? i.e macro in macro name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I believe it is possible, and I see where you're going with that question - it would allow us to eliminate the conditional in this case, but if you look at how %{python_sitelib} and %{python3_sitelib} are conditionalized elsewhere in the spec file, it wouldn't be consistent.

In other words, we could do this near the top of the spec file: %global _python_sitelib %{python%{_python_buildid}_sitelib} and then replace %python_sitelib and %python3_sitelib with %_python_sitelib. And that would allow us to get rid of the ceph_disk and ceph_volume conditionals, but none of the others - e.g. https://github.com/ceph/ceph/blob/master/ceph.spec.in#L1160-L1168

I'd rather keep the usage consistent throughout the spec file.

@smithfarm
build/ops: rpm: Python 3-only ceph-disk and ceph-volume
3d5dabf 
This should have been part of 661ccdb
but was overlooked.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
cmake: empty INSTALL_RPATH for libcls_sdk.so.1.0.0
d01d69e 
See 2354488 for explanation.

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

Passes Shaman test: https://shaman.ceph.com/builds/ceph/wip-python3-mopup/d01d69e511347495b793fff4bfb9ef875c905891/

@alfredodeza
Copy link
Contributor

jenkins test ceph-volume lvm all

@alfredodeza
Copy link
Contributor

jenkins test ceph-volume tox

@smithfarm smithfarm mentioned this pull request Jan 31, 2018
Merged
@tchaikov
Copy link
Contributor

tchaikov commented Feb 2, 2018

@tchaikov tchaikov merged commit af20ea9 into ceph:master Feb 2, 2018
@smithfarm smithfarm deleted the wip-python3-mopup branch February 2, 2018 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@ktdreyer ktdreyer Awaiting requested review from ktdreyer

Assignees
No one assigned
Labels
build/ops feature needs-qa wip-kefu-testing
Projects
None yet
Milestone
No milestone
3 participants
@smithfarm @alfredodeza @tchaikov