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

Disable building Proton by default #2362

Merged
merged 1 commit into from May 13, 2017
Merged

Disable building Proton by default #2362

merged 1 commit into from May 13, 2017

Conversation

daira
Copy link
Contributor

@daira daira commented May 10, 2017

fixes #2361

Signed-off-by: Daira Hopwood daira@jacaranda.org

@daira daira added this to the 1.0.9 milestone May 10, 2017
@daira
Disable building Proton by default.
b04529f 
Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@daira daira force-pushed the 2361.disable-proton-by-default branch from d583056 to b04529f Compare May 10, 2017 15:55
@nathan-at-least nathan-at-least mentioned this pull request May 10, 2017
Merged
nathan-at-least
nathan-at-least suggested changes May 10, 2017
View reviewed changes
Copy link
Contributor

@nathan-at-least nathan-at-least left a comment

Choose a reason for hiding this comment

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

We must ensure that CI does not pass non-default flags and therefore this PR merge will exercise the default build which should now exclude proton.

zcutil/build.sh
then
PROTON_ARG='--enable-proton=no'
PROTON_ARG=''
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: I suspect many users do ./configure …; make; make install, and I'd like to change the default for them. However, I'm content to rely on the underspecified policy that "build.sh is the only officially supported build path for Zcash" (despite my desire for #58 eventually).

@bitcartel
Copy link
Contributor

Note that #2280 should be included in 1.0.9 because it fixes potential build problems for people who do want to build proton. Otherwise they will opt in to build and be confused as to why it doesn't.

str4d
str4d approved these changes May 11, 2017
View reviewed changes
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK. I'd also like the argument to be inverted in configure.ac for consistency, but that's a soft requirement.

nathan-at-least
nathan-at-least approved these changes May 11, 2017
View reviewed changes
Copy link
Contributor

@nathan-at-least nathan-at-least left a comment

Choose a reason for hiding this comment

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

I have reviewed CI to verify that it builds with default args (aside from -j).

@nathan-at-least
Copy link
Contributor

@zkbot r+

@zkbot
Copy link
Contributor

zkbot commented May 11, 2017

📌 Commit b04529f has been approved by nathan-at-least

@zkbot
Copy link
Contributor

zkbot commented May 11, 2017

⌛ Testing commit b04529f with merge 48fcde310c8457ea73bf7c0bdb8f1fcc66787393...

@zkbot
Copy link
Contributor

zkbot commented May 11, 2017

💔 Test failed - pr-merge

@nathan-at-least
Copy link
Contributor

Tests failed because I manually cancelled the build, due to a problem with #2174.

@nathan-at-least
Copy link
Contributor

Eh, just clicked 'rebuild' on buildbot. Let's see how homu likes that.

@nathan-at-least
Copy link
Contributor

The answer is Homu does not like that, so:

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

⌛ Testing commit b04529f with merge ab711beedc0a540a593866418385f2a1e83e55a0...

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

💔 Test failed - pr-merge

@nathan-at-least
Copy link
Contributor

@zkbot retry

(I've confused buildbot by stopping and restarting various builders without regard to their triggering dependencies.)

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

⌛ Testing commit b04529f with merge 79283e2509be760ee5b41ba6ddd78db89600dda6...

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

💔 Test failed - pr-merge

@nathan-at-least
Copy link
Contributor

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

⌛ Testing commit b04529f with merge a61c949...

zkbot added a commit that referenced this pull request May 12, 2017
@zkbot
Auto merge of #2362 - daira:2361.disable-proton-by-default, r=nathan-…
a61c949 
…at-least

Disable building Proton by default

fixes #2361

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

💔 Test failed - pr-merge

@nathan-at-least
Copy link
Contributor

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

⌛ Testing commit b04529f with merge 80857feb5cdcfa5d61bedd8269678806328c6609...

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

💔 Test failed - pr-merge

@str4d
Copy link
Contributor

str4d commented May 12, 2017

Test failed because the builder couldn't access the network parameters.

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented May 12, 2017

⌛ Testing commit b04529f with merge 74cae51b0ff1433e7b3f3edc58c74e5a59eace6d...

@zkbot
Copy link
Contributor

zkbot commented May 13, 2017

💔 Test failed - pr-merge

@str4d
Copy link
Contributor

str4d commented May 13, 2017

The very last test failed because the builder didn't have pyzmq installed 😂

@zkbot retry

@zkbot
Copy link
Contributor

zkbot commented May 13, 2017

⌛ Testing commit b04529f with merge 2869db7...

zkbot added a commit that referenced this pull request May 13, 2017
@zkbot
Auto merge of #2362 - daira:2361.disable-proton-by-default, r=nathan-…
2869db7 
…at-least

Disable building Proton by default

fixes #2361

Signed-off-by: Daira Hopwood <daira@jacaranda.org>
@zkbot
Copy link
Contributor

zkbot commented May 13, 2017

☀️ Test successful - pr-merge
Approved by: nathan-at-least
Pushing 2869db7 to master...

@zkbot zkbot merged commit b04529f into zcash:master May 13, 2017
@daira daira deleted the 2361.disable-proton-by-default branch April 14, 2022 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bitcartel bitcartel bitcartel approved these changes

@nathan-at-least nathan-at-least nathan-at-least approved these changes

@str4d str4d str4d approved these changes

Assignees

@daira daira

Labels
None yet
Projects
No open projects
Continuous Improvement
  
Awaiting triage
Milestone
1.0.9
Development

Successfully merging this pull request may close these issues.

Swap proton build feature from opt-out (--disable-proton) to opt-in (--enable-proton)
5 participants
@daira @bitcartel @nathan-at-least @zkbot @str4d