0bc666b053 doc: add info for debugging with relative paths (S3RK)
a8b515c317 configure: keep relative paths in debug info (S3RK)
Pull request description:
This is a follow-up for #20353 that fixes#21885
It also adds a small section to assist debugging without absolute paths in debug info.
ACKs for top commit:
kallewoof:
Tested ACK 0bc666b053
Zero-1729:
Light crACK 0bc666b053
Tree-SHA512: d4b75183c3d3a0f59fe786841fb230581de87f6fe04cf7224e4b89c520d45513ba729d4ad8c0e62dd1dbaaa7a25741f04d036bc047f92842e76c9cc31ea47fb2
05f9770c1f doc: Clarify developer notes about constant naming (Russell Yanofsky)
Pull request description:
I'm pretty sure developer notes were intended to say constants should be upper case and variables should be lower case, but right now they are ambiguous about whether to write:
```c++
extern const int SYMBOL;
```
or:
```c++
extern const int g_symbol;
```
First convention above is better than the second convention because it tells you without having to look anything up that the value of `SYMBOL` won't change at runtime. Also I haven't seen other c++ projects using the second convention.
ACKs for top commit:
MarcoFalke:
cr ACK 05f9770c1f
practicalswift:
ACK 05f9770c1f
jarolrod:
ACK 05f9770c1f🥃
Tree-SHA512: 766d0e25d9db818d45df4ad6386987014f2053584cbced4b755ceef8bda6b7e2cfeb34eb8516423bd03b140faaf577614d5e3be2799f7eed0eb439187ab85323
Since the flag has been removed from the RPC command table in commit
ec6902d0ea (PR #11179), this guideline
is not relevant anymore and can be removed.
a4a3fc4cd2 doc: improve subtree check instructions (Sjors Provoost)
Pull request description:
Running `git-subtree-check.sh` requires adding the subtree repository as a remote. I learned that several years ago and then forgot again.
This PR also improves the error message if the subtree commit can't be found.
ACKs for top commit:
laanwj:
ACK a4a3fc4cd2
fanquake:
ACK a4a3fc4cd2 - this looks ok.
Tree-SHA512: 959bd923726c172d17f9f97f8a56988bf2df5a94d3131e5152a66150b941394cee9e82fdc6b86e09c0ba91d123a496599f07ca454212168d8d301738394c12c8
I'm pretty sure developer notes were intended to say constants should be upper
case and variables should be lower case, but right now they are ambiguous about
whether to write:
```c++
// foo.h
extern const int SYMBOL;
// foo.cpp
const int SYMBOL = 1;
```
or:
```c++
// foo.h
extern const int g_symbol;
// foo.cpp
const int g_symbol = 1;
```
First convention above is better than the second convention because it tells
you without having to look anything up that the value of `SYMBOL` will never
change at runtime. Also I've never seen any c++ project anywhere using the
second convention
3e32499909 Change example addresses to bech32 (Yusuf Sahin HAMZA)
Pull request description:
This is a follow-up PR to #18197 that fixes RPCExamples.
Fixes#18185.
ACKs for top commit:
MarcoFalke:
ACK 3e32499909
jonatack:
ACK 3e32499
Tree-SHA512: c7a6410ef8b6e169016c2c5eac3e6b9501caabd0e8a0871ec31e56bfc44589f056d3f5cb55b5a13bba36f6c15136c2352f883e30e4dcc0997ffd36b27f9173b9
155a11f897 doc: Added running functional tests in valgrind (Elichai Turkel)
Pull request description:
Technically the notes only show an "example" of how to run valgrind with the suppression file,
but now that https://github.com/bitcoin/bitcoin/pull/17633 is merged then maybe this can encourage more people to run also the functional tests in valgrind
Top commit has no ACKs.
Tree-SHA512: b8417249b720d0ed5e10b732648f2e07e8889bfc7aa7e94192d1c049b4b7837971678d30c535f273c227848f1290cf11e14369fd6c1924b734f2e47e2af41401
also contains the following changes:
- rpc: factor out example bech32 address for RPCExamples
- doc: update developer notes wrt RPCExamples addresses
(mention the EXAMPLE_ADDRESS constant as an example for an invalid bech32
address suitable for RPCExamples help documentation)
42ec499489 doc: developer notes guideline on RPCExamples addresses (Jon Atack)
Pull request description:
to make explicit the use of invalid addresses for user safety and to encourage
the use of bech32 addresses by default. See https://github.com/bitcoin/bitcoin/pull/17578#discussion_r361752570 and https://github.com/bitcoin/bitcoin/pull/17578#discussion_r362564492.
Fix a typo to appease the linter.
ACKs for top commit:
promag:
ACK 42ec499489, no strong opinion as whether this belongs to developer notes or not but why not.
fjahr:
ACK 42ec499
michaelfolkson:
ACK 42ec499489
Tree-SHA512: 64f90e227d256aa194c4fd48435440bdc233a51213dd4a6ac5b05d04263f729c6b4bb5f3afd3b87719b20cb1b159d5a9673d58a11b72823a4a6a16e8a26ae10e
to make explicit the use of invalid addresses for user safety and to encourage
the use of bech32 addresses by default.
Fix a typo to appease the linter.
794fe91395 doc: Update and improve Developer Notes (Hennadii Stepanov)
Pull request description:
This PR:
- removes outdated things, e.g., global pointer `pwalletMain` etc
- adds "Sanitizers" to the TOC
- makes filenames, `peer.dat` and `debug.log`, monospaced
- specifies that _compile-time_ constant names are all uppercase
- rewords using `explicit` with constructors
ACKs for top commit:
jamesob:
lazy ACK 794fe91395
practicalswift:
ACK 794fe91395 -- nice improvements!
Tree-SHA512: 2c5f035b1627f5fac8dc2453199d9e46bd101f86771de567cd95698de3c61cc459444ec1a68710e1d280195e1e40b42d9f40906297d12f12bf37749eca58297d
104f7de593 remove old bootstrap relevant code (tryphe)
Pull request description:
This picks up #15954
I fixed the code and added at a functional test utilizing the scripts in `contrib/linearize` as suggested by @MarcoFalke .
ACKs for top commit:
laanwj:
ACK 104f7de593
Tree-SHA512: acac9f285f9785fcbc3afc78118461e45bec2962f90ab90e9f82f3ad28adc90a44f0443b712458ccf486e46d891eb8a67f53e7bee5fa6d89e4387814fe03f117
9452802480 doc: Tidy up shadowing section (João Barbosa)
Pull request description:
Removes the example because it violates the code format.
ACKs for top commit:
MarcoFalke:
unsigned ACK 9452802480
ryanofsky:
ACK 9452802480
fanquake:
ACK 9452802480 - Thanks for following up.
Tree-SHA512: 1fc31355d368225713298da7803e39e99014fbfcd229f2d3b56c082de95ab2965e51c80b172a5abce4646c53f845fa62a6d94d5df714e7835cac07a8ec7d5da7
This pull request is regarding minor grammar and punctuation errors in the developer notes. There were no modifications to the existing code, only alterations to fix the grammar and punctuation in the text.
Update developer-notes.md
Update developer-notes.md
Revert "Update developer-notes.md"
This reverts commit dfeb0bacb054ed24766f8af7bae0c0166b0cb4cb.
The "Git and GitHub tips" section was moved from doc/developer-notes.md to doc/productivity.md in 5b76c31, but the header link to that long-gone section in the developer notes remains and needs to go.
So long, Git and GitHub tips, we barely knew ya.
After this commit, the only remaining output is:
$ test/lint/lint-spelling.sh
src/test/base32_tests.cpp:14: fo ==> of, for
src/test/base64_tests.cpp:14: fo ==> of, for
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt
Note:
* I ignore several valid alternative spellings
* homogenous is present in tinyformat, hence should be addressed upstream
* process' is correct only if there are plural processes
5b76c314d6 doc: Add separate productivity notes document (Carl Dong)
Pull request description:
Many developers have their own tools and tricks to be more productive
during their cycles, so let's document the best ones so that everyone
can benefit from them.
Tree-SHA512: b4989e7a815e972a9a646f448fb6c08bd896b4bce77fd7fb22a71a7602971d4cbe34f88183f503f5b851d002784d9e91b87df5348c661eeb9cefa69c52e0de2b
13782b8ba8 docs: add perf section to developer docs (James O'Beirne)
58180b5fd4 tests: add utility to easily profile node performance with perf (James O'Beirne)
Pull request description:
Adds a context manager to easily (and selectively) profile node performance during functional test execution using `perf`.
While writing some tests, I encountered some odd bitcoind slowness. I wrote up a utility (`TestNode.profile_with_perf`) that generates performance diagnostics for a node by running `perf` during the execution of a particular region of test code.
`perf` usage is detailed in the excellent (and sadly unmerged) https://github.com/bitcoin/bitcoin/pull/12649; all due props to @eklitzke.
### Example
```python
with node.profile_with_perf("large-msgs"):
for i in range(200):
node.p2p.send_message(some_large_msg)
node.p2p.sync_with_ping()
```
This generates a perf data file in the test node's datadir (`/tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data`).
Running `perf report` generates nice output about where the node spent most of its time while running that part of the test:
```bash
$ perf report -i /tmp/testtxmpod0y/node0/node-0-TestName-large-msgs.perf.data --stdio \
| c++filt \
| less
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 135 of event 'cycles:pp'
# Event count (approx.): 1458205679493582
#
# Children Self Command Shared Object Symbol
# ........ ........ ............... ................... ........................................................................................................................................................................................................................................................................
#
70.14% 0.00% bitcoin-net bitcoind [.] CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
|
---CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
70.14% 0.00% bitcoin-net bitcoind [.] CNetMessage::readData(char const*, unsigned int)
|
---CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
35.52% 0.00% bitcoin-net bitcoind [.] std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
|
---std::vector<char, zero_after_free_allocator<char> >::_M_fill_insert(__gnu_cxx::__normal_iterator<char*, std::vector<char, zero_after_free_allocator<char> > >, unsigned long, char const&)
CNetMessage::readData(char const*, unsigned int)
CNode::ReceiveMsgBytes(char const*, unsigned int, bool&)
...
```
Tree-SHA512: 9ac4ceaa88818d5eca00994e8e3c8ad42ae019550d6583972a0a4f7b0c4f61032e3d0c476b4ae58756bc5eb8f8015a19a7fc26c095bd588f31d49a37ed0c6b3e
Many developers have their own tools and tricks to be more productive
during their cycles, so let's document the best ones so that everyone
can benefit from them.
a62e667296 docs: Add more Doxygen information to Developer Notes (Jon Layton)
Pull request description:
Update information about Doxygen in `doc/developer-notes.md`.
Alternatively, this could have its own file (like `doc/web-documentation.md`), since there are installation steps included.
For example, I had to run:
```
brew install doxygen graphviz
```
on MacOS, otherwise failures occurred.
This information could also be linked to the `doc/release-process.md`.
Tree-SHA512: 5d77ee83e1b96fde036482b502f676a90a56f3f667753545a7cfba5c2e3b825644bb4cf0f8a84b7f9ba92fa5f2e1cd6ef1e27a94277f43d012355df741f7dd2f
a67d71311d [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](https://github.com/bitcoin/bitcoin/pull/14164#issuecomment-419752678) 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
b6718e373e tests: Use MakeUnique to construct objects owned by unique_ptrs (practicalswift)
Pull request description:
A subset of #14211 ("Use MakeUnique to construct objects owned by unique_ptrs") as suggested by @MarcoFalke in https://github.com/bitcoin/bitcoin/pull/14211#issuecomment-423324019.
Use `MakeUnique` to construct objects owned by `unique_ptr`s.
Rationale:
* `MakeUnique` ensures exception safety in complex expressions.
* `MakeUnique` gives a more concise statement of the construction.
Tree-SHA512: 1228ae6ce7beb178d79142c4e936b728178ccaa8aa35c6d8feeb33d1a667abfdd010c59996a9d833594611e913877ce5794e75953d11d9b1fdbac04aa491d9cf
a9e898a4ad docs: Link to python style guidelines from developer notes (Mason Simon)
Pull request description:
Initially I moved the python style guidelines from the functional test README, but some of the python rules are test-specific, and most of the developer notes doc is C++ centric, so just dropping a link seemed better.
Tree-SHA512: 9d4d5cc45526319a118595d90fcfad2c9aced22007aa096d8af04ba1b963312822804f4c15b0b227d66af49565034437691b7760e7ff6d1e3f8b10b898906362
e56771365b Do not use uppercase characters in source code filenames (practicalswift)
419a1983ca docs: Add a note about the source code filename naming convention (practicalswift)
Pull request description:
Add a note about the source code filename naming convention.
Tree-SHA512: 8d329bd9e19bcd26e74b0862fb0bc2369b46095dbd3e69d34859908632763abd7c3d00ccc44ee059772ad4bae4460c2bcc1c0e22fd9d8876d57e5fcd346cea4b
16e3cd380a Clarify include recommendation (practicalswift)
6d10f43738 Enforce the use of bracket syntax includes ("#include <foo.h>") (practicalswift)
906bee8e5f Use bracket syntax includes ("#include <foo.h>") (practicalswift)
Pull request description:
When analysing includes in the project it is often assumed that the preferred bracket include syntax (`#include <foo.h>`) mentioned in `developer-docs.md` is used consistently. @sipa:s excellent circular dependencies script [`circular-dependencies.py`](50c69b7801/contrib/devtools/circular-dependencies.py) (#13228) is an example of a script making this reasonable assumption.
This PR enables automatic Travis checking of the include syntax making sure that the bracket syntax includes (`#include <foo.h>`) is used consistently.
Tree-SHA512: a414921aabe8e487ebed42f3f1cbd02fecd1add385065c1f2244cd602c31889e61fea5a801507ec501ef9bd309b05d3c999f915cec1c2b44f085bb0d2835c182
Suggested formatting for namespaces in the developer guide is currently
inconsistent. This commit updates the developer guide and clang-format
configuration to consistently put "namespace" and opening/closing braces
on the same line. Example:
```c++
namespace boost {
namespace signals2 {
class connection;
} // namespace signals2
} // namespace boost
```
Currently the "Source code organization" section has an example like the one
above, but the "Coding style" section example and description put a newline
between the opening "namespace foo" and brace (but oddly no newline between
closing namespace and brace).
Avoiding newlines before namespace opening braces makes nested declarations
less verbose and also avoids asymmetry with closing braces. It's also a
common style used in other codebases:
* https://google.github.io/styleguide/cppguide.html#Namespaces
* https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Classes
* https://llvm.org/docs/CodingStandards.html#namespace-indentation
d1b622b tests: Add check for test suite name uniqueness in lint-tests.sh (practicalswift)
dc8067b tests: Add note about uniqueness requirement for test suite names (practicalswift)
3ebfb2d tests: Avoid test suite name collision in wallet crypto_tests (MarcoFalke)
Pull request description:
* Add documentation: Add note about test suite name uniqueness requirement in developer notes
* Add regression test: Update `lint-tests.sh` to make it check also for test suite name uniqueness
Context: #12894 (`tests: Avoid test suite name collision in wallet crypto_tests`)
Tree-SHA512: 3c8502db069ef3d753f534976a86a997b12bac539e808a7285193bf81c9dd8c1b06821c3dd1bdf870ab87722b02c8aa9574c62ace70c2a1b8091785cb8c9aace