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

Update univalue subtree #14164

Merged
merged 2 commits into from Sep 10, 2018

Conversation

Projects
None yet
7 participants
@MarcoFalke
Copy link
Member

commented Sep 7, 2018

This removes the deprecated std::pair wrappers from univalue, so that they are not accidentally re-introduced in our code base.

MarcoFalke added some commits Sep 7, 2018

Squashed 'src/univalue/' changes from 51d3ab34ba..7890db99d6
7890db99d6 Merge #11: Remove deprecated std pair wrappers
40e34852ac Merge #14: Cleaned up namespace imports to reduce symbol collisions
85052a4819 Remove deprecated std::pair wrappers
d208f986dd Cleaned up namespace imports to reduce symbol collisions

git-subtree-dir: src/univalue
git-subtree-split: 7890db99d693572d27ade3e14268bd7236134529
@fanquake

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

utACK faad55a

Ran make check and tested the subtree merge.

merge 14164
[pull/14164/local-merge a83101646] Merge #14164: Update univalue subtree
 Date: Fri Sep 7 20:55:42 2018 +0800
#14164 Update univalue subtree into master
* faad55a9a Update univalue subtree (MarcoFalke) (pull/14164/head)
* dc287c98f Squashed 'src/univalue/' changes from 51d3ab34ba..7890db99d6 (MarcoFalke)

Dropping you on a shell so you can try building/testing the merged source.
Run 'git diff HEAD~' to show the changes being merged.
Type 'exit' when done.
bash-3.2$ git fetch https://github.com/bitcoin-core/univalue.git
warning: no common commits
remote: Counting objects: 604, done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 604 (delta 0), reused 1 (delta 0), pack-reused 602
Receiving objects: 100% (604/604), 125.21 KiB | 265.00 KiB/s, done.
Resolving deltas: 100% (332/332), done.
From https://github.com/bitcoin-core/univalue
 * branch                HEAD       -> FETCH_HEAD
bash-3.2$ ./test/lint/git-subtree-check.sh src/univalue
src/univalue in HEAD currently refers to tree e73765406fc0a0fb4796504ea08a2e05cfea8bc0
src/univalue in HEAD was last updated in commit dc287c98f8b33576f3c71db02a232106ef0e57d9 (tree e73765406fc0a0fb4796504ea08a2e05cfea8bc0)
src/univalue in HEAD was last updated to upstream commit 7890db99d693572d27ade3e14268bd7236134529 (tree e73765406fc0a0fb4796504ea08a2e05cfea8bc0)
GOOD
@jnewbery

This comment has been minimized.

Copy link
Member

commented Sep 7, 2018

tACK faad55a

@DrahtBot

This comment has been minimized.

Copy link
Contributor

commented Sep 7, 2018

Note to reviewers: This pull request conflicts with the following ones:
  • #12435 (RPC: Strict JSON-RPC 2.0 compliance (gated behind flag) by esotericnonsense)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@promag

This comment has been minimized.

Copy link
Member

commented Sep 9, 2018

utACK faad55a.

@luke-jr
Copy link
Member

left a comment

NACK, upstream is 88967f658663654a67f1cda93ca6f99de1f7c91f

Referenced 7890db99d693572d27ade3e14268bd7236134529 does not exist upstream

@promag

This comment has been minimized.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Sep 9, 2018

Upstream is jgarzik/univalue

bitcoin-core/univalue should not exist. There is no rationale for deviating from upstream, and if anything, we should be working toward removing the subtree entirely and just using the system install.

@ken2812221

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

utACK faad55a.

@luke-jr At least, it's a fork for now.

@luke-jr

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

There's no reason for it, so NACK making it so...

@jnewbery

This comment has been minimized.

Copy link
Member

commented Sep 10, 2018

Upstream has been bitcoin-core/univalue since at least #8807.

This PR doesn't change what the upstream branch is, so discussion of that seems off-topic for me.

ACK faad55a

MarcoFalke added a commit to MarcoFalke/bitcoin that referenced this pull request Sep 10, 2018

Merge bitcoin#14164: Update univalue subtree
dc287c9 Squashed 'src/univalue/' changes from 51d3ab34ba..7890db99d6 (MarcoFalke)

Pull request description:

  This removes the deprecated `std::pair` wrappers from univalue, so that they are not accidentally re-introduced in our code base.

Tree-SHA512: 46f6f7c7c7942a9e131d971c425cbde4159abcc1214235b61139ce97b174024e47b9c52e832cde89fbab9879f43d12c26b64d6def9907bd47883f1e574cf2e2e

@MarcoFalke MarcoFalke merged commit faad55a into bitcoin:master Sep 10, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MarcoFalke MarcoFalke deleted the MarcoFalke:Mf1809-univalueSubtree branch Sep 10, 2018

MarcoFalke added a commit that referenced this pull request Dec 6, 2018

Merge #14882: [doc] developer-notes.md: point out that UniValue devia…
…tes from upstream

a67d713 [doc] developer-notes.md: point out that UniValue deviates from upstream (Sjors Provoost)

Pull request description:

  While debugging an issue I was somewhat surprised to [learn](#14164 (comment)) that we've moved `src/univalue` from https://github.com/jgarzik/univalue to https://github.com/bitcoin-core/univalue, that these repos are both maintained and they're different.

  The first mention of using the bitcoin-core repo is from late 2015 in #7157. I didn't check when the last common ancestor commit is.

  I couldn't find documentation as to why (these things just happen in open source of course), but at minimum we should make this more clear.

  There's also the following line in `config.ac` that I'm not sure what to do with:
  ```
  AC_INIT([univalue], [1.0.3],
          [http://github.com/jgarzik/univalue/])
  ```

Tree-SHA512: e58105677b5ebe0005772282da4a805fee7dfccacfb1b2686a874517bf46072d1481181f8a8865d25526f6ed9e5fcd55d8d49906bf27cd0f5aefe4f258aa4d63

eabz pushed a commit to polispay/polis that referenced this pull request Feb 14, 2019

Cronos

eabz pushed a commit to polispay/polis that referenced this pull request Feb 14, 2019

Cronos
Revert "Backport bitcoin/pull/14164"
This reverts commit 162e743.
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.