There is no justification for forking UniValue. The subtree should be restored to the official upstream and the fork deleted.
Bitcoin Core should not have a fork of univalue #15009
issue luke-jr opened this issue on December 20, 2018-
luke-jr commented at 2:50 AM on December 20, 2018: member
- fanquake added the label Build system on Dec 20, 2018
-
jnewbery commented at 3:54 PM on December 20, 2018: member
This is a complete waste of time. You've NACK'ed a bunch of useful PRs (https://github.com/bitcoin-core/univalue/pull/11#issuecomment-419453702, #14164#pullrequestreview-153596070, etc) because you believe the upstream should be jgarzik/univalue without once giving a justification of why that should be.
jgarzik/univalue appears to be very infrequently maintained. There haven't been any code changes since August, and a request to add a 1.0.4 release (prompted by you here: #12666) has been unacknowledged since August 21st.
Switching to having our own repo was a pragmatic decision to avoid relying on a spottily-maintained single-maintainer repo, as Wladimir rightly points out here: #14882 (comment).
jgarzik does not appear to be interested in actively maintaining his repo. Why should we add that dependency and demand that he maintain it for us?
-
luke-jr commented at 5:51 PM on December 20, 2018: member
Not-forking should be the default; only forking should require justification.
It hasn't needed any code changes since August AFAICT.
Despite the stale open issue, v1.0.4 was released Sep 1: https://github.com/jgarzik/univalue/releases/tag/v1.0.4
cc @jgarzik
-
MarcoFalke commented at 6:59 PM on December 20, 2018: member
Note that the repo is missing a test fix, that we needed: https://github.com/jgarzik/univalue/pull/57 (created pull request per next comment)
Also our copy has the deprecated pair wrappers removed to avoid introducing them in future code changes and to avoid having to constantly fix them up again. I think I explained that earlier. Are we running in circles here?
-
luke-jr commented at 11:54 PM on December 20, 2018: member
Where is the test fix PR upstream? That is an example of why we shouldn't have a fork...
-
jgarzik commented at 3:44 PM on December 21, 2018: contributor
- univalue is actively maintained upstream (jgarzik/univalue)
- To the specific issue of deprecated pair wrappers, upstream has had - for a long time now - a method of compiling those out, to provide a certainty that future Bitcoin code cannot use them.
- fanquake added the label Upstream on Jan 10, 2019
-
MarcoFalke commented at 11:24 PM on May 8, 2020: member
The feature request didn't seem to attract much attention in the past. Also, the issue seems not important enough right now to keep it sitting around idle in the list of open issues.
Closing due to lack of interest. Pull requests with improvements are always welcome.
- MarcoFalke closed this on May 8, 2020
- DrahtBot locked this on Feb 15, 2022