Skip to content

univalue: Pull subtree #8863

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
Merged
laanwj merged 2 commits into from
Oct 3, 2016
Merged

univalue: Pull subtree #8863

laanwj merged 2 commits into from
Oct 3, 2016

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Oct 2, 2016

MarcoFalke added 2 commits September 30, 2016 19:58
Squashed 'src/univalue/' changes from daf1285..16a1f7f
2ca7faa 
16a1f7f Merge bitcoin#3: Pull upstream
3f03bfd Merge pull request #27 from laanwj/2016_09_const_refs
5668ca3 Return const references from getKeys, getValues, get_str
cedda14 Merge pull request #28 from MarcoFalke/patch-1
9f0b997 [travis] Work around osx libtool issue

git-subtree-dir: src/univalue
git-subtree-split: 16a1f7f6e9ed932bec7c7855003affea1e165fb5
Merge commit '2ca7faab4205822b06dc2ab2bbda0a9a70fce7e0' into HEAD
e757115
@laanwj
Copy link
Member

laanwj commented Oct 2, 2016 

$ contrib/devtools/git-subtree-check.sh src/univalue
src/univalue in HEAD was last updated to upstream commit 16a1f7f6e9ed932bec7c7855003affea1e165fb5 (tree fe714bb1bdfbd52842107b54c590cfe5d4c58b3c)
src/univalue in HEAD currently refers to tree fe714bb1bdfbd52842107b54c590cfe5d4c58b3c
GOOD

ACK 2ca7faa

paveljanik
paveljanik reviewed Oct 2, 2016
View reviewed changes
src/univalue/.travis.yml
- pkg-config

before_script:
- if [[ "$TRAVIS_OS_NAME" == "osx" ]]; then brew uninstall libtool; brew install libtool; fi
Copy link
Contributor

@paveljanik paveljanik Oct 2, 2016

Choose a reason for hiding this comment

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

What you are trying to workaround? Where can I read more about this?

Copy link
Member Author

@maflcko maflcko Oct 2, 2016

Choose a reason for hiding this comment

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

See the upstream repo :)

Copy link
Contributor

@paveljanik paveljanik Oct 3, 2016

Choose a reason for hiding this comment

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

The upstream issue (jgarzik/univalue#28) is of a somewhat discutable quality. Another reference to the outside source. I just want to know what is the source of the problem that is solved by uninstall/install round...

Copy link
Member

@laanwj laanwj Oct 3, 2016

Choose a reason for hiding this comment

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

The install will install a newer/different version of libtool which doesn't have an issue that prevented compiling the univalue project on macosx. I'd expect it to be a temporary workaround.
Has this been reported meta-upstream to Travis?

Copy link
Member Author

@maflcko maflcko Oct 3, 2016

Choose a reason for hiding this comment

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

@paveljanik The issue with homebrew was considered "mostly fixed" (Homebrew/legacy-homebrew#43874 (comment)) by some of the workarounds. If the maintainers of that project don't consider it that imporatnt, I think it is fine to just apply a trivial and low risk workaround to fix a travis issue.

Also note that the content of this .travis.yml is of no relevance in the bitcoin repo.

@paveljanik
Copy link
Contributor

paveljanik commented Oct 2, 2016

utack

@laanwj laanwj merged commit e757115 into bitcoin:master Oct 3, 2016
laanwj added a commit that referenced this pull request Oct 3, 2016
@laanwj
Merge #8863: univalue: Pull subtree
eafc5f4 
2ca7faa Squashed 'src/univalue/' changes from daf1285..16a1f7f (MarcoFalke)
@maflcko maflcko deleted the Mf1610-univalue branch October 3, 2016 08:25
@laanwj
Copy link
Member

laanwj commented Oct 3, 2016

!!IMPORTANT!!

If you end up here with the following error in test_bitcoin:

Running 210 test cases...
unknown location(0): fatal error in "base58_EncodeBase58": memory access violation at address: 0x00000016: no mapping at fault address
/home/orion/projects/bitcoin/bitcoin/src/test/base32_tests.cpp(21): last checkpoint

You need to clean your tree and build from scratch. Apparently changes to univalue are not detected by the build system, so files that depend on univalue.h will be rebuilt but not the library itself. This causes ABI conflicts.

@kyuupichan kyuupichan mentioned this pull request Apr 30, 2017
Merged
codablock pushed a commit to codablock/dash that referenced this pull request Jan 12, 2018
@laanwj @codablock
Merge bitcoin#8863: univalue: Pull subtree
10127ae 
2ca7faa Squashed 'src/univalue/' changes from daf1285..16a1f7f (MarcoFalke)
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 5, 2019
Update univalue
2d43ec0 
bitcoin#8807
bitcoin#8863
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
@laanwj @andvgal
Merge bitcoin#8863: univalue: Pull subtree
697766b 
2ca7faa Squashed 'src/univalue/' changes from daf1285..16a1f7f (MarcoFalke)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@laanwj laanwj laanwj left review comments

+1 more reviewer

@paveljanik paveljanik paveljanik left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Upstream

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@maflcko @laanwj @paveljanik