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
LTTng-UST disabled for openSUSE #10592
Conversation
%if 0%{?is_opensuse} | ||
ExclusiveArch x86_64 aarch64 ppc64 ppc64le | ||
%else | ||
ExclusiveArch x86_64 aarch64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need a ':' after 'ExclusiveArch' I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed those changes as not related to
http://tracker.ceph.com/issues/16937
@michelmno Please look into interactive rebasing to redo the PR, i.e. Lots of tutorials out there on that - for example, https://www.atlassian.com/git/tutorials/rewriting-history/ You can use that to squash commits into one, drop commits, etc. |
@michelmno Basically, when modifying a PR you always use |
%if 0%{?fedora} || 0%{?rhel} >= 6 || 0%{?suse_version} >= 1315 | ||
%if ! 0%{?is_openSUSE} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
%if 0%{?fedora} || 0%{?rhel} >= 6 || ( 0%{?suse_version} == 1315 && ! 0%{?is_openSUSE} )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, since ceph is only built for SLE 12, there's little point in checking explicitly for 1315. The following would be sufficient:
%if 0%{?fedora} || 0%{?rhel} >= 6 || ( 0%{?suse_version} && ! 0%{?is_opensuse} )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a comment in (1) saying:
"Grouping with parentheses will fail on fedora/rhel/centos"
This is the reason why I used successive %if in my patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt if the parentheses are still broken in RH/CentOS/Fedora, but if you like we can leave it separate as you suggest. How about this, then:
%if 0%{?fedora} || 0%{?rhel} >= 6 || 0%{?suse_version}
%if ! 0%{?is_opensuse}
(note the lowercase is_opensuse)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if you'd rather leave >= 1315
and SLES12+
that's OK with me.
@michelmno Looks good, now please squash the three commits into one. |
@michelmno There is an unwritten rule that downstream bug tracker numbers (bz#, bsc#) don't belong in upstream commit messages. Please amend the commit message so it doesn't mention the downstream bug. It's enough to say that lttng doesn't work in openSUSE and so it has to be disabled there. |
LTTng-UST not yet supported in openSUSE so do not enable lltng for it. The (1) is where is defined "is_opensuse" Remove value for test of suse_version in spec file and change related comment from SLES12 to SLE as per comment in #10592 (1) https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto#Detect_a_distribution_flavor_for_special_code Fixes: http://tracker.ceph.com/issues/16937 Signed-off-by: Michel Normand <normand@linux.vnet.ibm.com>
pushed to gitbuilder as |
LTTng-UST not yet supported in openSUSE so do not enable lltng for it. The (1) is where is defined "is_opensuse" Remove value for test of suse_version in spec file and change related comment from SLES12 to SLE as per comment in ceph#10592 (1) https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto#Detect_a_distribution_flavor_for_special_code Fixes: http://tracker.ceph.com/issues/16937 Signed-off-by: Michel Normand <normand@linux.vnet.ibm.com> (cherry picked from commit 7da19b6)
LTTng-UST not yet supported in openSUSE so do not enable lltng for it. The (1) is where is defined "is_opensuse" Remove value for test of suse_version in spec file and change related comment from SLES12 to SLE as per comment in ceph#10592 (1) https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto#Detect_a_distribution_flavor_for_special_code Fixes: http://tracker.ceph.com/issues/16937 Signed-off-by: Michel Normand <normand@linux.vnet.ibm.com> (cherry picked from commit 7da19b6)
LTTng-UST not yet supported in openSUSE so do not enable lltng for it. The (1) is where is defined "is_opensuse" Remove value for test of suse_version in spec file and change related comment from SLES12 to SLE as per comment in ceph#10592 (1) https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto#Detect_a_distribution_flavor_for_special_code Fixes: http://tracker.ceph.com/issues/16937 Signed-off-by: Michel Normand <normand@linux.vnet.ibm.com>
as per suse_version definition from url:
https://en.opensuse.org/openSUSE:Build_Service_cross_distribution_howto#Detect_a_distribution_flavor_for_special_code
Fixes: http://tracker.ceph.com/issues/16937
Signed-off-by: Michel Normand normand@linux.vnet.ibm.com
@smithfarm comments ?