doc: rewrite dependencies.md

fanquake commented at 11:52 am on November 21, 2021: member

This PR rewrites dependencies.md. The current list is hard to parse, includes information that is either incorrect and/or misleading, and duplicates info in other documentation. The list of dependencies is much smaller, because it’s now just the actual dependencies of Bitcoin Core, not random Qt things, or the dependencies of other tooling. We don’t need another section on configure flag usage, or, to have duplicated lists of dependencies in other build docs, as that somewhat defeats the point of having dependencies.md, and just means more effort keeping things in sync.

fanquake added the label Docs on Nov 21, 2021

in doc/dependencies.md:11 in 73ee8ddaad outdated

61+
62+When compiling against `-stdlib=libc++`, version 7.0 or later is required.
63+
64+| Compiler | Minimum required |
65+| --- | --- |
66+| [Clang](https://clang.llvm.org) | [7.0](https://github.com/bitcoin/bitcoin/pull/23060) |

katesalazar commented at 12:06 pm on November 21, 2021:

I’d love tables free from links:

0| Compiler   | Minimum required |
1| ---------- | ---------------- |
2| [Clang][0] | [7.0][1]         |

0[0]: https://clang.llvm.org
1[1]: [#23060](/bitcoin-bitcoin/23060/)

Much better than:

0| Compiler | Minimum required |
1| --- | --- |
2| [Clang](https://clang.llvm.org) | [7.0](https://github.com/bitcoin/bitcoin/pull/23060) |

jamesob commented at 4:57 pm on November 22, 2021:

Not necessary IMO, and matching up the indexes may get confusing if there is a long list of footnotes. Might be better if the indexes could be strings and not numbers, but still unnecessary IMO.

mjdietzx commented at 3:14 pm on November 21, 2021: contributor

Concept ACK - nice cleanup

jarolrod commented at 4:49 am on November 22, 2021: member

Concept ACK

to have duplicated lists of dependencies in other build docs

I had originally duplicated the lists in the other build docs, besides build-unix.md. Since we have dependencies.md, I’d be in agreement of keeping dependencies there. My idea with the dependencies list in the build docs was to keep most of the information that a user needs to build contained within the build doc itself, and not need to jump around. I understand that this approach can be an unnecessary maintenance cost, especially when these dependencies change.

Do you want to go ahead and remove the dependency tables from the other build-*.md docs?

fanquake force-pushed on Nov 22, 2021

fanquake commented at 7:01 am on November 22, 2021: member

Do you want to go ahead and remove the dependency tables from the other build-*.md docs?

Have done.

naumenkogs commented at 7:09 am on November 22, 2021: member

Concept ACK

DrahtBot commented at 12:59 pm on November 22, 2021: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#24031 (build: don’t compress macOS DMG by fanquake)

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.

in doc/dependencies.md:33 in e57aca9359 outdated

79+## Optional
80+
81+### GUI
82+| Dependency | Version used | Minimum required | Runtime |
83+| --- | --- | --- | --- |
84+| [Fontconfig](https://www.freedesktop.org/wiki/Software/fontconfig/) | 2.12.1 | 2.6 | Yes |

shaavan commented at 2:19 pm on November 22, 2021:

nit: Since the package name is fontconfig. I would suggest updating the name to:

0| [fontconfig](https://www.freedesktop.org/wiki/Software/fontconfig/) | 2.12.1 | 2.6 | Yes |

katesalazar commented at 8:11 pm on November 22, 2021:

Even its own web site uses capitalized and non capitalized randomly. Use whatever fits other rows.

shaavan commented at 2:20 pm on November 22, 2021: contributor

Concept ACK

Cleanup looks good. The list displays all the significant dependencies for compiling Bitcoin Core.
I have checked that the information provided in update dependencies.md is unchanged from the master’s dependencies.md.
MiniUPnPc minimum version required info is added. Unfortunately, I cannot test whether this information is correct as I am on Ubuntu 20.04, and the package version below 2.1.20 is not available on it.
The links are correct and direct to the right appropriate web pages.

in doc/build-freebsd.md:13 in e57aca9359 outdated

 8-
 9-The following dependencies are **required**:
10-
11- Library                                                               | Purpose    | Description
12- ----------------------------------------------------------------------|------------|----------------------
13- [autoconf](https://svnweb.freebsd.org/ports/head/devel/autoconf/)     | Build      | Automatically configure software source code

jamesob commented at 5:21 pm on November 22, 2021:

Looks like none of the autotools stuff is mentioned in the new deps section - do we just expect that’s pulled in by some other dependency?

jarolrod commented at 9:20 pm on November 22, 2021:

No, it is not drawn in by another dependency. This is a situation that highlights that no approach is perfect.

The current duplicated tables have certain specific dependencies that only apply to that platform. With the approach introduced in this PR, you’d have to list the dependency and note that it applies for X platform only.

fanquake commented at 0:30 am on November 23, 2021:

Looks like none of the autotools stuff is mentioned in the new deps section

It isn’t, and I don’t think it needs to be. It’s not a dependency of Bitcoin Core, just like git, or hexdump isn’t. They are build tools, and the installation instructions for them are is the build docs.

The current duplicated tables have certain specific dependencies that only apply to that platform.

That’s a rarity, and I think it’s fine for those to just be mentioned in the build docs where relevant.

katesalazar commented at 5:32 am on November 23, 2021:

Looks like none of the autotools stuff is mentioned in the new deps section

It isn’t, and I don’t think it needs to be. It’s not a dependency of Bitcoin Core, just like git, or hexdump isn’t. They are build tools, and the installation instructions for them are is the build docs.

autotools are build tools; when no alternative to them, they are build dependencies

fanquake commented at 3:10 pm on February 10, 2022:

autoconf and automake are added now

in doc/build-osx.md:30 in e57aca9359 outdated

25-[berkeley-db@4](https://formulae.brew.sh/formula/berkeley-db@4) | Berkeley DB      | Wallet storage (only needed when wallet enabled)
26-[qt@5](https://formulae.brew.sh/formula/qt@5)                   | GUI              | GUI toolkit (only needed when GUI enabled)
27-[qrencode](https://formulae.brew.sh/formula/qrencode)           | QR codes in GUI  | Generating QR codes (only needed when GUI enabled)
28-[zeromq](https://formulae.brew.sh/formula/zeromq)               | ZMQ notification | Allows generating ZMQ notifications (requires ZMQ version >= 4.0.0)
29-[sqlite](https://formulae.brew.sh/formula/sqlite)               | SQLite DB        | Wallet storage (only needed when wallet enabled)
30-[miniupnpc](https://formulae.brew.sh/formula/miniupnpc)         | UPnP Support     | Firewall-jumping support (needed for port mapping support)

jamesob commented at 7:52 pm on November 22, 2021:

So far as I can tell, for deps like miniupnpc we no longer have the “Description” information living here. Might be nice to add a column to the dependencies.md table that lists the info here.

jamesob commented at 7:52 pm on November 22, 2021: member

Concept ACK

RandyMcMillan commented at 9:09 pm on November 22, 2021: contributor

https://github.com/bitcoin/bitcoin/pull/23565/files/e57aca9359bdbb8813c0bea9df92e81b97dfe00d#diff-e9ba5bd8b63e72aa2d54d02960c9209823938981691cc8786ba323b54de0659cR53

bison needs to be installed for the /depends build. (yacc) Maybe include it here for completeness?

jarolrod commented at 9:14 pm on November 22, 2021: member

@RandyMcMillan the mentioned build docs and this PR are for dynamic builds, not static builds. The bison dependency for static builds is already documented under the /depends readme.

RandyMcMillan commented at 9:33 pm on November 22, 2021: contributor

@jarolrod - I get it - but to reduce documentation complexity further - we could download everything for a static build - then it is just a matter of enumerating the different commands for dynamic or static builds. Each platform would need one build doc.

Rspigler commented at 5:39 am on December 3, 2021: contributor

Concept ACK. Cleanup is nice

DrahtBot added the label Needs rebase on Dec 3, 2021

fanquake force-pushed on Jan 10, 2022

DrahtBot removed the label Needs rebase on Jan 10, 2022

DrahtBot added the label Needs rebase on Jan 10, 2022

fanquake force-pushed on Feb 10, 2022

DrahtBot removed the label Needs rebase on Feb 10, 2022

in doc/dependencies.md:22 in 9d50ed23e7 outdated

71+
72+| Dependency | Version used | Minimum required | Runtime |
73+| --- | --- | --- | --- |
74+| [Boost](https://www.boost.org/users/download/) | 1.71.0 | [1.64.0](https://github.com/bitcoin/bitcoin/pull/22320) | No |
75+| [libevent](https://github.com/libevent/libevent/releases) | 2.1.12-stable | [2.0.21](https://github.com/bitcoin/bitcoin/pull/18676) | No |
76+| [glibc](https://www.gnu.org/software/libc/) | N/A | [2.18](https://github.com/bitcoin/bitcoin/pull/23511) | Yes |

luke-jr commented at 3:35 am on February 11, 2022:

AFAIK 2.18 is only required for the release binaries, not the minimum required.

fanquake commented at 10:46 am on February 22, 2022:

It could potentially be 2.16, if you build with --disable-threadlocal, however I think using 2.18, which aligns with the release binairies, is fine here.

in doc/dependencies.md:11 in 9d50ed23e7 outdated

54+
55+## Build, Debug & Tests
56+
57+A compiler with support for C++17 and `std::filesystem` is required.
58+
59+When compiling against `-stdlib=libc++`, version 7.0 or later is required.

MarcoFalke commented at 10:28 am on February 22, 2022:

Not sure if this is needed. I think this should only be mentioned when the required libc++ version is larger than the required clang version. It should be self-explanatory that you can’t use a libc++ version smaller than your clang version.

fanquake commented at 10:32 am on February 22, 2022:

Agree. Dropped this line.

fanquake force-pushed on Feb 22, 2022

DrahtBot added the label Needs rebase on Feb 23, 2022

fanquake force-pushed on Mar 4, 2022

fanquake commented at 5:20 pm on March 4, 2022: member

Rebased.

DrahtBot removed the label Needs rebase on Mar 4, 2022

fanquake force-pushed on Mar 7, 2022

fanquake commented at 2:56 pm on March 7, 2022: member

Rebased past #24132.

DrahtBot added the label Needs rebase on Mar 12, 2022

fanquake force-pushed on Mar 12, 2022

fanquake commented at 8:26 pm on March 12, 2022: member

Rebased past #24164.

DrahtBot removed the label Needs rebase on Mar 12, 2022

doc: rework dependencies.md 893e18059f

in doc/build-freebsd.md:30 in 791030c8f1 outdated

25-  Library                                                                    | Purpose          | Description
26-  ---------------------------------------------------------------------------|------------------|----------------------
27-  [db5](https://svnweb.freebsd.org/ports/head/databases/db5/)                | Berkeley DB      | Wallet storage (only needed when wallet enabled)
28-  [qt5](https://svnweb.freebsd.org/ports/head/devel/qt5/)                    | GUI              | GUI toolkit (only needed when GUI enabled)
29-  [libqrencode](https://svnweb.freebsd.org/ports/head/graphics/libqrencode/) | QR codes in GUI  | Generating QR codes (only needed when GUI enabled)
30-  [libzmq4](https://svnweb.freebsd.org/ports/head/net/libzmq4/)              | ZMQ notification | Allows generating ZMQ notifications (requires ZMQ version >= 4.0.0)

MarcoFalke commented at 8:57 am on March 13, 2022:

I think it is useful to know the name of the optional dependency. Maybe add a new line on how to install all the optional dependencies?

0pkg install libzmq4 ...

fanquake commented at 10:19 am on March 16, 2022:

I’ve added a line for libzmq4, all of the others are already mentioned.

fanquake force-pushed on Mar 16, 2022

MarcoFalke approved

MarcoFalke commented at 10:31 am on March 16, 2022: member

cr ACK 893e18059f1f92a254aa2026312a769c0e711db6

Nice cleanup

hebasto approved

hebasto commented at 3:13 pm on March 16, 2022: member

ACK 893e18059f1f92a254aa2026312a769c0e711db6, I have reviewed the code and it looks OK, I agree it can be merged.

I’ve verified links, package versions – both in depends, and minimum required. GUI dependencies verified against Qt docs.

jarolrod commented at 3:25 pm on March 16, 2022: member

crACK https://github.com/bitcoin/bitcoin/commit/893e18059f1f92a254aa2026312a769c0e711db6

fanquake merged this on Mar 16, 2022

fanquake closed this on Mar 16, 2022

fanquake deleted the branch on Mar 16, 2022

fanquake commented at 3:46 pm on March 16, 2022: member

Thanks for the reviews. Planning on open some further doc consolidation PRs shortly.

sidhujag referenced this in commit 0160360c22 on Mar 16, 2022

MarcoFalke referenced this in commit e09cf64c48 on Mar 17, 2022

fanquake referenced this in commit 1c6fcea205 on Apr 21, 2022

sidhujag referenced this in commit 758b31b0d3 on Apr 22, 2022

DrahtBot locked this on Mar 16, 2023

doc: rewrite dependencies.md #23565

Conflicts