depends: add -g to DEBUG=1 flags #29527

fanquake · 2024-03-01T16:36:46Z

Add -g to the base DEBUG=1 flags in depends.
Avoids the need to specify it per-package.
More alignment with --enable-debug behaviour in configure.

We also want to align the optimization flags, currently -O1 vs -O0, however that can be it's own PR.

DrahtBot · 2024-03-01T16:36:50Z

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	theuni
Concept ACK	TheCharlatan, hebasto

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

theuni · 2024-03-01T16:45:30Z

Concept ACK.

As I've mentioned before, I don't have a strong preference for -O0 vs -O1 vs -Og, but -Og makes the most sense to me.

More alignment with --enable-debug behaviour in configure.
We also want to align the optimization flags, currently -O1 vs -O0, however that can be it's own PR.

This I strongly agree with though. Whatever we choose we should use everywhere.

theuni · 2024-03-01T16:48:31Z

If I recall, the original reason for keeping -g out of here is that it seriously blows up the size of the build, which (at the time) had very real consequences for caching with c-i. Unsure if it's as big a deal these days?

maflcko · 2024-03-12T10:10:27Z

The CI depends cache is "unlimited" in size, and the CI ccache size can be modified, if needed.

TheCharlatan · 2024-03-13T20:47:02Z

Concept ACK

TheCharlatan · 2024-03-13T21:25:04Z

I get an error in the debug windows build:

make HOST=x86_64-w64-mingw32 DEBUG=1 -j 30
...
Qt Tools:
  QDoc ................................... yes

Note: Also available for Linux: linux-clang linux-icc

Note: Using static linking will disable the use of dynamically
loaded plugins. Make sure to import all needed static plugins,
or compile needed modules into the library.

ERROR: Qt requires a compliant STL library.
make: *** [funcs.mk:291: /home/drgrid/bitcoin/depends/x86_64-w64-mingw32/.qt_stamp_configured] Error 3

hebasto · 2024-03-18T13:33:34Z

Concept ACK.

Add -g to the base DEBUG=1 flags in depends.
More alignment with --enable-debug behaviour in configure.

Why not -g3 then?

bitcoin/configure.ac

Line 353 in 015ac13

dnl Prefer -g3, fall back to -g if that is unavailable.

fanquake · 2024-03-18T13:34:35Z

Why not -g3 then?

Because we aren't testing, and -g always works.

fanquake · 2024-03-26T14:16:48Z

I get an error in the debug windows build:

Looks like that error is unrelated to the changes here, and instead the Windows Qt DEBUG=1 build is just broken on master.

The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`. I'm guessing this has been broken since bitcoin#24131 (01d1845), but have not checked. Fixes bitcoin#29734. Unblocks bitcoin#29527 (now DEBUG=1 builds can be tested).

fanquake · 2024-03-27T11:02:57Z

Looks like that error is unrelated to the changes here, and instead the Windows Qt DEBUG=1 build is just broken on master.

Reabsed on a commit that fixes this.

b7e7e72 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake) Pull request description: The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`. I'm guessing this has been broken since #24131 (01d1845), but have not checked. Fixes #29734. Unblocks #29527 (`DEBUG=1` builds can be tested). ACKs for top commit: hebasto: ACK b7e7e72, tested on Ubuntu 22.04 with the [installed](#29734 (comment)) `g++-mingw-w64-x86-64` package. TheCharlatan: ACK b7e7e72 Tree-SHA512: 9e24e84046c0489c20971bb9c68d1a643c233837193c184f61bff79dfc8d7397a5c5526ac1a205ad423920f2589559cd01cb104ceb7f89515bb6421510d82ca9

theuni

ACK 84fbf9b

fanquake · 2024-04-03T09:46:02Z

Going to followup with an optimization flags alingment RFC.

The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`. I'm guessing this has been broken since bitcoin#24131 (01d1845), but have not checked. Fixes bitcoin#29734. Unblocks bitcoin#29527 (now DEBUG=1 builds can be tested). Github-Pull: bitcoin#29747 Rebased-From: b7e7e72

84fbf9b depends: remove -g from sqlite debug flags (fanquake) eef51af depends: add -g to DEBUG=1 flags (fanquake) Pull request description: Add `-g` to the base DEBUG=1 flags in depends. Avoids the need to specify it per-package. More alignment with `--enable-debug` behaviour in configure. We also want to align the optimization flags, currently -O1 vs -O0, however that can be it's own PR. ACKs for top commit: theuni: ACK 84fbf9b Tree-SHA512: 1ee98ba0c13e4b80bb87632658b4f53ce49c73e0e7712990c30da60deca4a349a744232f2d78f243dee9a07f5b9b70f9c2c4ae34082c34ae51b37b054fac61fd

b7e7e72 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake) Pull request description: The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`. I'm guessing this has been broken since bitcoin#24131 (01d1845), but have not checked. Fixes bitcoin#29734. Unblocks bitcoin#29527 (`DEBUG=1` builds can be tested). ACKs for top commit: hebasto: ACK b7e7e72, tested on Ubuntu 22.04 with the [installed](bitcoin#29734 (comment)) `g++-mingw-w64-x86-64` package. TheCharlatan: ACK b7e7e72 Tree-SHA512: 9e24e84046c0489c20971bb9c68d1a643c233837193c184f61bff79dfc8d7397a5c5526ac1a205ad423920f2589559cd01cb104ceb7f89515bb6421510d82ca9

700b8c5 Merge bitcoin#29658: Bugfix: GUI: Help messages already have a trailing newline, so don't add an extra one (merge-script) a0cd305 Merge bitcoin#29948: test: add missing comparison of node1's mempool in MempoolPackagesTest (Ava Chow) f1907ea Merge bitcoin#29984: net: Replace ifname check with IFF_LOOPBACK in Discover (merge-script) daa6eee Merge bitcoin#29960: depends: pass verbose through to cmake based makefiles (merge-script) 61a5832 Merge bitcoin#29907: test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos (merge-script) 23f25a9 Merge bitcoin#29872: test: Add missing Assert(mock_time_in >= 0s) to SetMockTime (merge-script) a7daee7 Merge bitcoin#29689: lint: scripted-diff verification also requires GNU grep (Ava Chow) 3df1ca1 Merge bitcoin#29953: doc: Bash is needed in gen_id and is not installed on FreeBSD by default (merge-script) b53b854 Merge bitcoin#29850: net: Decrease nMaxIPs when learning from DNS seeds (Ava Chow) c4a147c Merge bitcoin#28340: security: restrict abis in bitcoind.service (Ryan Ofsky) acfdf9e Merge bitcoin#28373: doc: Add example of mixing private and public keys in descriptors (Ava Chow) 51bc8bd Merge bitcoin#29859: build: Fix false positive `CHECK_ATOMIC` test (merge-script) bb4102c Merge bitcoin#29893: test: fix intermittent failure in p2p_compactblocks_hb.py (glozow) 4ecb761 Merge bitcoin#29786: Drop Windows Socket dependency for `randomenv.cpp` (fanquake) 1a8e805 Merge bitcoin#29498: test: Update --tmpdir doc string to say directory must not exist (fanquake) 81ca71c Merge bitcoin#29781: depends: add new LLVM debug macro (fanquake) 5ce92ca Merge bitcoin#29527: depends: add -g to DEBUG=1 flags (fanquake) Pull request description: ## Issue being fixed or feature implemented Trivial backports ## What was done? ## How Has This Been Tested? built locally ## Breaking Changes ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 700b8c5 but pls confirm Guix is happy now before merging kwvg: utACK 700b8c5 Tree-SHA512: 9722979c4f0589cb02bfeaf39373713372a4aa1c8c5a55aa5b4d33388f73ef19231de3963b80ffaad25fbe0db90a133de5080baac691daed41b86a762b867b2d

b7e7e72 depends: fix mingw-w64 Qt DEBUG=1 build (fanquake) Pull request description: The issue is that compilation is done with `x86_64-w64-mingw32-g++-posix`, but then linking is done with `x86_64-w64-mingw32-g++`. I'm guessing this has been broken since bitcoin#24131 (01d1845), but have not checked. Fixes bitcoin#29734. Unblocks bitcoin#29527 (`DEBUG=1` builds can be tested). ACKs for top commit: hebasto: ACK b7e7e72, tested on Ubuntu 22.04 with the [installed](bitcoin#29734 (comment)) `g++-mingw-w64-x86-64` package. TheCharlatan: ACK b7e7e72 Tree-SHA512: 9e24e84046c0489c20971bb9c68d1a643c233837193c184f61bff79dfc8d7397a5c5526ac1a205ad423920f2589559cd01cb104ceb7f89515bb6421510d82ca9

fanquake requested a review from theuni March 1, 2024 16:36

DrahtBot added the Build system label Mar 1, 2024

fanquake mentioned this pull request Mar 26, 2024

qt: Windows build with DEBUG=1 is broken #29734

Closed

fanquake mentioned this pull request Mar 27, 2024

depends: fix mingw-w64 Qt DEBUG=1 build #29747

Merged

fanquake force-pushed the depends_g_debug_flags branch from fd09ed7 to 4675b2b Compare March 27, 2024 10:57

fanquake force-pushed the depends_g_debug_flags branch 2 times, most recently from 7eed316 to aa78b9a Compare April 1, 2024 15:54

DrahtBot added the CI failed label Apr 1, 2024

fanquake added 2 commits April 2, 2024 17:12

depends: add -g to DEBUG=1 flags

eef51af

depends: remove -g from sqlite debug flags

84fbf9b

fanquake force-pushed the depends_g_debug_flags branch from aa78b9a to 84fbf9b Compare April 2, 2024 16:12

theuni approved these changes Apr 2, 2024

View reviewed changes

DrahtBot requested review from TheCharlatan and hebasto April 2, 2024 16:33

fanquake merged commit 0d509ba into bitcoin:master Apr 3, 2024

fanquake deleted the depends_g_debug_flags branch April 3, 2024 09:43

Pttn added a commit to Pttn/Bitcoin that referenced this pull request Jan 10, 2025

Merge bitcoin#29527

fa223e7

bitcoin locked and limited conversation to collaborators Apr 3, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

depends: add -g to DEBUG=1 flags #29527

depends: add -g to DEBUG=1 flags #29527

Uh oh!

fanquake commented Mar 1, 2024 •

edited

Loading

Uh oh!

DrahtBot commented Mar 1, 2024 •

edited

Loading

Uh oh!

theuni commented Mar 1, 2024

Uh oh!

theuni commented Mar 1, 2024

Uh oh!

maflcko commented Mar 12, 2024

Uh oh!

TheCharlatan commented Mar 13, 2024

Uh oh!

TheCharlatan commented Mar 13, 2024

Uh oh!

hebasto commented Mar 18, 2024

Uh oh!

fanquake commented Mar 18, 2024

Uh oh!

fanquake commented Mar 26, 2024 •

edited

Loading

Uh oh!

fanquake commented Mar 27, 2024

Uh oh!

theuni left a comment

Uh oh!

fanquake commented Apr 3, 2024

Uh oh!

Uh oh!

depends: add -g to DEBUG=1 flags #29527

depends: add -g to DEBUG=1 flags #29527

Uh oh!

Conversation

fanquake commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Mar 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage

Reviews

Uh oh!

theuni commented Mar 1, 2024

Uh oh!

theuni commented Mar 1, 2024

Uh oh!

maflcko commented Mar 12, 2024

Uh oh!

TheCharlatan commented Mar 13, 2024

Uh oh!

TheCharlatan commented Mar 13, 2024

Uh oh!

hebasto commented Mar 18, 2024

Uh oh!

fanquake commented Mar 18, 2024

Uh oh!

fanquake commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Mar 27, 2024

Uh oh!

theuni left a comment

Choose a reason for hiding this comment

Uh oh!

fanquake commented Apr 3, 2024

Uh oh!

Uh oh!

fanquake commented Mar 1, 2024 •

edited

Loading

DrahtBot commented Mar 1, 2024 •

edited

Loading

fanquake commented Mar 26, 2024 •

edited

Loading