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.

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

Dependency Updates #3809

Merged
merged 5 commits into from Feb 15, 2019

Conversation

@defuse
Copy link
Contributor

commented Jan 23, 2019

This updates:

  • Boost from 1.66.0 to 1.69.0
  • OpenSSL From 1.1.0h to 1.1.1a
  • Proton from 0.17.0 to 0.26.0
  • Rust from 1.28.0 to 1.32.0
@daira

This comment has been minimized.

Copy link
Contributor

commented Jan 27, 2019

NACK for the e7fdbfd commit since it would break RPC tests on macOS.

@defuse defuse force-pushed the defuse:dependency-updates branch from 31cd50e to aa99366 Jan 30, 2019

@defuse defuse changed the title [WIP] Dependency Updates Dependency Updates Jan 30, 2019

@defuse

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2019

This is ready for review. I rebased on top of the current master and removed the commit @daira NACKed (#3799 will fix it for me). It's best to review depends/patches/proton/minimal-build.patch on its own, otherwise you're looking at a diff between two diffs. I may not have properly redone the patching work that was done in #2280, since I don't fully understand the motivation aside from fixing the one build issue. I would prefer to keep the patch small (or even remove it if possible) so that updating Proton is easier in the future. Someone should download the tarballs from a different vantage point and verify that the hashes match. Here are the URLs to save you time:

You might also want to look over my summaries of the changelogs in #3786 to see if I've missed any changes that will negatively impact us somehow. I'm not up to speed on our conventions for commit messages or how the PR title / first comment should be, so feel free to be pedantic about that.

Once merged, https://z.cash/depends-sources/ needs to be updated with the new files.

This doesn't quite close #3786, since there are still other dependencies that haven't been updated and I haven't determined whether there are any security fixes in those ones. This fixes #3816.

@str4d str4d added the dependencies label Jan 30, 2019

@str4d str4d added this to Needs Prioritization in Arborist Team via automation Jan 30, 2019

@str4d str4d moved this from Needs Prioritization to PRs That Need Review + Their Associated Issues in Arborist Team Jan 30, 2019

@mms710 mms710 requested review from str4d and bitcartel Jan 31, 2019

@mms710 mms710 moved this from PRs That Need Review + Their Associated Issues to In Progress in Arborist Team Jan 31, 2019

@mms710 mms710 moved this from In Progress to In Review in Arborist Team Jan 31, 2019

@bitcartel

This comment has been minimized.

Copy link
Contributor

commented Jan 31, 2019

Whilst folk are reviewing, let's see how the various builders do...

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Jan 31, 2019

⌛️ Trying commit aa99366 with merge 9be82ac...

zkbot added a commit that referenced this pull request Jan 31, 2019

Auto merge of #3809 - defuse:dependency-updates, r=<try>
Dependency Updates

This updates:

- Boost from 1.66.0 to 1.69.0
- OpenSSL From 1.1.0h to 1.1.1a
- Proton from 0.17.0 to 0.26.0
- Rust from 1.28.0 to 1.32.0
@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 1, 2019

💔 Test failed - pr-try

@str4d

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

ACK up to f3d9977 - I have verified the hashes on my machine.

NACK aa99366 - CI fails specifically on the Proton-enabled builder, and passes on the other builders.

btest:

unknown location(0): fatal error: in "Alert_tests/AlertApplies": Throw location unknown (consider using BOOST_THROW_EXCEPTION)
Dynamic exception type: boost::wrapexcept<boost::signals2::no_slots_error>
std::exception::what: boost::signals2::no_slots_error
test/alert_tests.cpp(283): last checkpoint: "AlertApplies" fixture ctor
Running stage no-dot-so
=======================
libqpid-proton-cpp.so.12
libqpid-proton-core.so.10
libqpid-proton.so.11
libqpid-proton-core.so.10.6.0
libqpid-proton-cpp.so.12.3.0
libqpid-proton-core.so
libqpid-proton-cpp.so
libqpid-proton-proactor.so.1.3.4
libqpid-proton-proactor.so.1
libqpid-proton.so.11.7.0
libqpid-proton-proactor.so
libqpid-proton.so
FAIL.
------------------------
Finished stage no-dot-so

RPC tests failed due to low disk space; I assume related to the btest failures (which maybe generated coredumps?)

@str4d

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

For those who don't want to go searching through Buildbot's logs, here is the part where Proton gets built:

3809-proton-build-log.txt

@str4d
Copy link
Contributor

left a comment

Proton dependency needs fixing.

@str4d

This comment has been minimized.

Copy link
Contributor

commented Feb 1, 2019

I think the no-dot-so failure is because -DBUILD_STATIC_LIBS=ON causes static libraries to be built in addition to the shared ones, not instead of. If we comment out this line in the patch, that might resolve itself.

@daira

daira approved these changes Feb 1, 2019

Copy link
Contributor

left a comment

utACK (nodulo fixes to Proton). I have not checked the hashes.

@defuse defuse self-assigned this Feb 5, 2019

@defuse defuse force-pushed the defuse:dependency-updates branch from e3eeced to 78c916e Feb 6, 2019

@defuse

This comment has been minimized.

Copy link
Contributor Author

commented Feb 6, 2019

I fixed the proton patch to not build the .so files, and now the no-dot-so test passes (I squashed all the proton commits into one and forced pushed). I'm not able to reproduce the btest failures locally on any of my VMs (including a Debian 8 one). The first error we see is...

*****unknown location(0): fatal error: in "Alert_tests/AlertApplies": Throw location unknown (consider using BOOST_THROW_EXCEPTION)
Dynamic exception type: boost::wrapexcept<boost::signals2::no_slots_error>
std::exception::what: boost::signals2::no_slots_error
test/alert_tests.cpp(283): last checkpoint: "AlertApplies" fixture ctor

...and the following ones all look like...

unknown location(0): fatal error: in "Alert_tests/AlertNotify": std::runtime_error: CDBEnv::MakeMock: Already initialized
test/alert_tests.cpp(324): last checkpoint: "AlertNotify" fixture ctor

After looking at this code this suggests an exception is being thrown in TestingSetup::TestingSetup() in/after the call to bitdb.MakeMock() or in the ReadAlerts : public TestingSetup constructor, and TestingSetup::~TestingSetup() isn't getting run which would explain the subsequent "MakeMock Already initialized" errors. The TestingSetup constructor creates some database files on disk so perhaps the disk was already full by then, but that doesn't explain why it's presenting as a boost::signals2::no_slots_error. I'll see if I can figure it out but in the meantime maybe we can retry the build.

@mms710 mms710 moved this from In Review to PRs That Need Review + Their Associated Issues in Arborist Team Feb 14, 2019

@mms710 mms710 moved this from PRs That Need Review + Their Associated Issues to Sprint Backlog in Arborist Team Feb 14, 2019

@mms710 mms710 moved this from Sprint Backlog to Blocked in Arborist Team Feb 14, 2019

@str4d

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 14, 2019

⌛️ Trying commit 78c916e with merge 3ebcb44...

zkbot added a commit that referenced this pull request Feb 14, 2019

Auto merge of #3809 - defuse:dependency-updates, r=<try>
Dependency Updates

This updates:

- Boost from 1.66.0 to 1.69.0
- OpenSSL From 1.1.0h to 1.1.1a
- Proton from 0.17.0 to 0.26.0
- Rust from 1.28.0 to 1.32.0
@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 14, 2019

☀️ Test successful - pr-try
State: approved= try=True

@str4d

str4d approved these changes Feb 15, 2019

Copy link
Contributor

left a comment

utACK. Proton issues are fixed.

@str4d str4d added this to the v2.0.4 milestone Feb 15, 2019

@str4d str4d moved this from Blocked to Merge Queue in Arborist Team Feb 15, 2019

@str4d

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 15, 2019

📌 Commit 78c916e has been approved by str4d

@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 15, 2019

⌛️ Testing commit 78c916e with merge 6a20252...

zkbot added a commit that referenced this pull request Feb 15, 2019

Auto merge of #3809 - defuse:dependency-updates, r=str4d
Dependency Updates

This updates:

- Boost from 1.66.0 to 1.69.0
- OpenSSL From 1.1.0h to 1.1.1a
- Proton from 0.17.0 to 0.26.0
- Rust from 1.28.0 to 1.32.0
@zkbot

This comment has been minimized.

Copy link
Collaborator

commented Feb 15, 2019

☀️ Test successful - pr-merge
Approved by: str4d
Pushing 6a20252 to master...

@zkbot zkbot merged commit 78c916e into zcash:master Feb 15, 2019

1 check passed

homu Test successful
Details

Arborist Team automation moved this from Merge Queue to Released (Merged in Master) Feb 15, 2019

zkbot added a commit that referenced this pull request Mar 25, 2019

Auto merge of #3918 - defuse:fix-proton-patch, r=bitcartel
Fix proton patch regression.

Closes #3916.  Fixes a regression introduced in #3809.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

zkbot added a commit that referenced this pull request Mar 26, 2019

Auto merge of #3918 - defuse:fix-proton-patch, r=<try>
Fix proton patch regression.

Closes #3916.  Fixes a regression introduced in #3809.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>

zkbot added a commit that referenced this pull request Mar 26, 2019

Auto merge of #3918 - defuse:fix-proton-patch, r=bitcartel
Fix proton patch regression.

Closes #3916.  Fixes a regression introduced in #3809.

Co-authored-by: Daira Hopwood <daira@jacaranda.org>
@bitcartel

This comment has been minimized.

Copy link
Contributor

commented Mar 26, 2019

For the v2.0.4 release, we had to patch this PR to fix build issues. See #3918 and #3919.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.