doc: Extend developer-notes with file-name-only debugging fix

l0rinc commented at 10:36 am on August 18, 2024: contributor

While testing the new cmake build with CLion, I noticed that the tests don’t always stop at the set breakpoints, so I’ve updated the developer-notes.md, hoping it will be useful for others experiencing the same.

Added links to gdb and lldb documentations.

Assumed a default directory (similarly to https://github.com/hebasto/bitcoin/pull/328/files#diff-4d2a64ce14cb8b971dbba9455421b04ae7ed0c489c66d983664be5632b0de4a3R19) to make the commands more realistic.

Extended the possible debugging fixes with file-name-only option.

DrahtBot commented at 10:36 am on August 18, 2024: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/30670.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	laanwj, achow101

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

DrahtBot added the label Docs on Aug 18, 2024

l0rinc force-pushed on Aug 18, 2024

DrahtBot commented at 10:41 am on August 18, 2024: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/28909204621

Make sure to run all tests locally, according to the documentation.

The failure may happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

DrahtBot added the label CI failed on Aug 18, 2024

DrahtBot removed the label CI failed on Aug 18, 2024

in doc/developer-notes.md:373 in 816e73e763 outdated

372 
373-For `gdb` create or append to `.gdbinit` file:
374+For `gdb` create or append to [`.gdbinit` file](https://sourceware.org/gdb/current/onlinedocs/gdb#gdbinit-man):
375 ```
376-set substitute-path ./src /path/to/project/root/src
377+set substitute-path ./src ~/bitcoin/src

glozow commented at 1:59 pm on August 20, 2024:

Doesn’t this just make the docs less generalizable? Why is that better?

l0rinc commented at 2:24 pm on August 20, 2024:

My take was that ~/bitcoin/src is easier to generalize to e.g. C:\\Users\\Username\\Documents\\bitcoin\\src than /path/to/project/root/src to C:\\Users\\Username\\Documents\\bitcoin\\src, since there we have to understand that root means bitcoin and src refers to the literal src folder.

My inspiration for using concrete examples was https://github.com/hebasto/bitcoin/pull/328/files#diff-4d2a64ce14cb8b971dbba9455421b04ae7ed0c489c66d983664be5632b0de4a3R19

If it’s not helpful, I don’t mind reverting.

maflcko commented at 6:18 am on August 21, 2024:

What you link to is about the build folder, not the source folder. IIUC suggesting to use the build folder build is due to the .gitignores taking effect. However, users are otherwise free to use any other build folder.

The same does not apply to the source folder, so making them “consistent” doesn’t apply here.

Also, even a small LLM like 4o-mini “understands” the previous doc correctly, so I am not sure what can be hard to understand.

I don’t see why this change is worth it.

have to understand that root means bitcoin

That isn’t true. It is possible to clone to any folder name, see git clone --help. This is also using the same format as the old docs, so leaving it as-is seems more consistent:

0       <directory>
1           The name of a new directory to clone into. The "humanish" part of the
2           source repository is used if no <directory> is explicitly given (repo for
3           /path/to/repo.git and foo for host.xz:foo/.git). Cloning into an existing
4           directory is only allowed if the directory is empty.

l0rinc commented at 8:35 am on August 21, 2024:

Valid points, thanks @glozow & @maflcko, dropped that commit.

l0rinc force-pushed on Aug 21, 2024

DrahtBot added the label CI failed on Sep 9, 2024

l0rinc force-pushed on Sep 9, 2024

DrahtBot removed the label CI failed on Sep 9, 2024

achow101 requested review from achow101 on Oct 15, 2024

achow101 requested review from m3dwards on Oct 15, 2024

DrahtBot added the label CI failed on Oct 23, 2024

achow101 commented at 9:45 pm on October 23, 2024: member

ACK d1546b3c4f24d3ff6abe729a4288e827786cf6e9

achow101 removed review request from achow101 on Oct 23, 2024

Add gdb and lldb links to debugging troubleshooting cb7c5ca824

l0rinc force-pushed on Oct 24, 2024

l0rinc commented at 10:37 am on October 24, 2024: contributor

Rebased to avoid unrelated build failure

DrahtBot removed the label CI failed on Oct 24, 2024

Extend possible debugging fixes with file-name-only 1b0b9b4c78

in doc/developer-notes.md:395 in 37d22d86e7 outdated

391@@ -392,6 +392,8 @@ ln -s /path/to/project/root/src src
392 
393 3. Use `debugedit` to modify debug information in the binary.
394 
395+4. Change your breakpoints to use the file name only.

laanwj commented at 9:47 am on October 29, 2024:

Maybe add something like “if the IDE has an option for this” to make it slightly more clear, out of context, that it’s an IDE option, and not something to set with say, debugedit.

l0rinc commented at 12:59 pm on October 29, 2024:

Done, thanks

l0rinc force-pushed on Oct 29, 2024

laanwj approved

laanwj commented at 1:07 pm on October 29, 2024: member

ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049

DrahtBot requested review from achow101 on Oct 29, 2024

achow101 commented at 8:54 pm on November 4, 2024: member

ACK 1b0b9b4c7873ff0c6323de0c7f439466eed06049

achow101 merged this on Nov 4, 2024

achow101 closed this on Nov 4, 2024

l0rinc deleted the branch on Nov 4, 2024

doc: Extend developer-notes with file-name-only debugging fix #30670

Code Coverage & Benchmarks

Reviews