-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Backported redistributable logic to Origin specfile #12969
Backported redistributable logic to Origin specfile #12969
Conversation
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.
Thank You Thank You
I was so busy when I found this I didn't have time to backport it.
It seems that when you wrap condition statements in that initial stuff (that this removes) nullifies all the condition statements. The only reason this was working before was because the statement we wanted was always last.
No problem! Lets merge this so the Friday merge is simpler |
[Test]ing while waiting on the merge queue |
Broke rpm build in gce |
@tdawson do you have cycles to re-evaluate the logic?
|
Looking into it. It should have built them all. |
OK, I found the problem. I'll work on a solution that works for both. |
As long as there is a way to indicate that we only want Linux binaries, we don't necessarily need the |
origin.spec
Outdated
@@ -40,7 +39,6 @@ | |||
%global make_redistributable 0 | |||
%endif | |||
%endif | |||
} | |||
|
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.
The following works in all cases.
Replace line 32-43 with the following
%if 0%{?fedora} || 0%{?epel}
%global need_redistributable_set 0
%else
# Due to library availability, redistributable builds only work on x86_64
%ifarch x86_64
%global need_redistributable_set 1
%else
%global need_redistributable_set 0
%endif
%endif
%{!?make_redistributable: %global make_redistributable %{need_redistributable_set}}
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 think that seems reasonable -- if you patch it into OSE I'll pick up the new commit in this PR
Will do |
This back-ports downstream commit 25fa795. Signed-off-by: Steve Kuznetsov <[email protected]>
414c465
to
ab709c2
Compare
@tdawson updated here [test] |
Evaluated for origin test up to ab709c2 |
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/358/) (Base Commit: e376a9a) |
LGTM ... and it passed the tests this time. :) |
[merge] |
1 similar comment
[merge] |
@stevekuznetsov We are not frozen. We are branching tomorrow morning. |
Oh jeez there was a sneaky merge queue comment at the top ... |
gotta love it when there are 24 comments, but the merge bot takes comment #4 to use to tell us it's merging. :) |
Evaluated for origin merge up to ab709c2 |
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_future/358/) (Base Commit: 3ef5377) (Image: devenv-rhel7_5945) |
This back-ports downstream commit 25fa795.
Signed-off-by: Steve Kuznetsov [email protected]