Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

bitcoin-core: remove no-longer needed DEBUG comment #9828

Merged
merged 1 commit into from
Apr 14, 2023
Merged

bitcoin-core: remove no-longer needed DEBUG comment #9828

merged 1 commit into from
Apr 14, 2023

Conversation

fanquake
Copy link
Contributor

@fanquake fanquake commented Feb 28, 2023 •
edited
Loading

We'll be removing the _LIBCPP_DEBUG (which has been deprecated/removed
by LLVM), downstream in bitcoin/bitcoin#27447.

So remove the comment about re-enabling DEBUG=1, as that will no-longer
do anything for the builds here.

We could follow up with getting a Debug Mode build of libc++ available in the
oss-fuzz environment.

@fanquake
Copy link
Contributor Author

cc @dergoegge

@fanquake
Copy link
Contributor Author

Build failures here looking spurious (...2f9f3b8b: no space left on device). Can we kick/reboot these whenever possible?

@jonathanmetzman
Copy link
Contributor

close and reopen to retry

@fanquake fanquake closed this Feb 28, 2023
@jonathanmetzman
Copy link
Contributor

jonathanmetzman commented Feb 28, 2023 •
edited
Loading

Or push (probably even force push)

@fanquake fanquake reopened this Feb 28, 2023
@fanquake fanquake force-pushed the re_enable_debug_in_depends branch from 7fd7c9e to b542874 Compare February 28, 2023 21:12
@fanquake
Copy link
Contributor Author

Or push (probably even force push)

Force pushed now.

@maflcko
Copy link
Contributor

maflcko commented Mar 1, 2023

Yeah, I think I fixed that one in llvm/llvm-project@faa019c

lgtm.

CI failure can be ignored

@maflcko
Copy link
Contributor

maflcko commented Mar 1, 2023

Ideas for followups would be:

@maflcko maflcko mentioned this pull request Mar 1, 2023
Merged
@jonathanmetzman
Copy link
Contributor

Do you want me to merge this now or wait until I make a call on the other one.

@fanquake fanquake force-pushed the re_enable_debug_in_depends branch from b542874 to 28c170b Compare March 2, 2023 20:45
@fanquake
Copy link
Contributor Author

fanquake commented Mar 2, 2023

Rebased past #9836.

@maflcko
Copy link
Contributor

maflcko commented Mar 3, 2023

Ah ok, that wasn't fixed yet. Apparently it is a build system problem, see llvm/llvm-project#53669 (comment)

@fanquake
Copy link
Contributor Author

fanquake commented Mar 8, 2023

Ah ok, that wasn't fixed yet. Apparently it is a build system problem,

Ok. I don't have time to look at this now, but can get to it next week. Looks like we might have to change how LLVM is compiled. Not sure how involved that will be too do here.

fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 11, 2023
@fanquake
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
5de5899 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
@fanquake fanquake mentioned this pull request Apr 11, 2023
Merged
fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 11, 2023
@fanquake
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
b092882 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
@fanquake
bitcoin-core: remove comment about DEBUG=1
619cb86 
We'll be removing the `_LIBCPP_DEBUG` (which has been deprecated/removed
by LLVM), downstream in bitcoin/bitcoin#27447.

So remove the comment about re-enabling DEBUG=1, as that will no-longer
do anything for the builds here.

We could follow up with getting a Debug Mode build of libc++ available in the
oss-fuzz environment.
@fanquake fanquake force-pushed the re_enable_debug_in_depends branch from 28c170b to 619cb86 Compare April 11, 2023 15:41
@maflcko
Copy link
Contributor

maflcko commented Apr 11, 2023

Thanks, we'll just run DEBUG=1 on our infra. Unless oss-fuzz also provides a libc++ built with debug enabled.

@fanquake
Copy link
Contributor Author

I've changed this to just remove the comment about DEBUG=1. See the commit message / updated PR description.

@jonathanmetzman is there interest in making a Debug Mode build of libc++ available in the oss-fuzz env? I've had a look, and couldn't see any usage of LIBCXX_ENABLE_DEBUG_MODE, and only 1 usage of D_LIBCPP_DEBUG in the bls-signatures project, so I assume this isn't currently available?

@jonathanmetzman
Copy link
Contributor

@kcc WDYT of this?

@jonathanmetzman jonathanmetzman enabled auto-merge (squash) April 11, 2023 16:17
fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 13, 2023
@fanquake
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
c1a906b 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
@jonathanmetzman
Copy link
Contributor

This change only removes a comment. Do we still want this merged?

@fanquake
Copy link
Contributor Author

@jonathanmetzman yes thanks.

@fanquake fanquake changed the title bitcoin-core: re-enable DEBUG=1 in the depends build bitcoin-core: remove no-longer needed DEBUG comment Apr 14, 2023
jonathanmetzman
jonathanmetzman approved these changes Apr 14, 2023
View reviewed changes
Copy link
Contributor

@jonathanmetzman jonathanmetzman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jonathanmetzman jonathanmetzman merged commit df76eb3 into google:master Apr 14, 2023
@fanquake fanquake deleted the re_enable_debug_in_depends branch April 14, 2023 15:01
fanquake added a commit to fanquake/bitcoin that referenced this pull request Apr 18, 2023
@fanquake
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
cf266b2 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
fanquake added a commit to bitcoin-core/gui that referenced this pull request Apr 19, 2023
@fanquake
Merge bitcoin/bitcoin#27447: depends: Remove _LIBCPP_DEBUG from dep…
d908877 
…ends DEBUG mode

bc4fd49 depends: add _LIBCPP_ENABLE_ASSERTIONS to DEBUG mode (fanquake)
cf266b2 depends: Remove _LIBCPP_DEBUG from depends DEBUG mode (fanquake)

Pull request description:

  It was deprecated in LLVM 15, turned into compile-time error in LLVM 16:
  ```bash
  In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
  /usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
  Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
      ^
  1 error generated.
  ```

  and has been removed entirely in LLVM 17 (main): llvm/llvm-project@ff573a4.

  [Building libc++ in debug mode](https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html), will also automatically set
  `_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
  doesn't seem useful, and would just result in redefinition errors.

  I'm wondering if as a followup, we could enable a DEBUG build of libc++
  in our MSAN CI job? i.e https://github.com/fanquake/bitcoin/tree/msan_with_enable_debug_mode.

  Somewhat related to google/oss-fuzz#9828, where
  it looks like we'll have to sort out getting a DEBUG build of LLVM, and can drop the commentary about re-enabling DEBUG=1.

ACKs for top commit:
  MarcoFalke:
    lgtm Approach ACK bc4fd49

Tree-SHA512: 9c0f48fc428278fbf34fbb8f81e761e232506d7ab28e971cb9a9b9a81d549b4d8bbe51e2f7608d56e489428679231da5b7431443849b238a8a993ad241740282
@fanquake fanquake mentioned this pull request Apr 19, 2023
Open
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 19, 2023
@fanquake
Merge bitcoin#27447: depends: Remove _LIBCPP_DEBUG from depends DEB…
dcc7090 
…UG mode

bc4fd49 depends: add _LIBCPP_ENABLE_ASSERTIONS to DEBUG mode (fanquake)
cf266b2 depends: Remove _LIBCPP_DEBUG from depends DEBUG mode (fanquake)

Pull request description:

  It was deprecated in LLVM 15, turned into compile-time error in LLVM 16:
  ```bash
  In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
  /usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
  Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
      ^
  1 error generated.
  ```

  and has been removed entirely in LLVM 17 (main): llvm/llvm-project@ff573a4.

  [Building libc++ in debug mode](https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html), will also automatically set
  `_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
  doesn't seem useful, and would just result in redefinition errors.

  I'm wondering if as a followup, we could enable a DEBUG build of libc++
  in our MSAN CI job? i.e https://github.com/fanquake/bitcoin/tree/msan_with_enable_debug_mode.

  Somewhat related to google/oss-fuzz#9828, where
  it looks like we'll have to sort out getting a DEBUG build of LLVM, and can drop the commentary about re-enabling DEBUG=1.

ACKs for top commit:
  MarcoFalke:
    lgtm Approach ACK bc4fd49

Tree-SHA512: 9c0f48fc428278fbf34fbb8f81e761e232506d7ab28e971cb9a9b9a81d549b4d8bbe51e2f7608d56e489428679231da5b7431443849b238a8a993ad241740282
RandyMcMillan pushed a commit to RandyMcMillan/bitcoin that referenced this pull request May 27, 2023
@fanquake @RandyMcMillan
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
0169dc7 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
janus pushed a commit to BitgesellOfficial/bitgesell that referenced this pull request Sep 3, 2023
@fanquake @janus
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
71bbd3e 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
backpacker69 pushed a commit to peercoin/peercoin that referenced this pull request Mar 5, 2024
@fanquake @backpacker69
depends: Remove _LIBCPP_DEBUG from depends DEBUG mode
0ed7064 
It was deprecated in LLVM 15, turned into a compile-time error in LLVM 16:
```bash
In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
/usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
    ^
1 error generated.
```

and has been removed entirely in LLVM 17 (main),
llvm/llvm-project@ff573a4.

Building libc++ in debug mode, will also automatically set
`_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
doesn't seem useful, and would just result in redefinition errors.

I'm wondering if as a followup, we could enable a DEBUG build of libc++
in our MSAN CI job?

Somewhat related to google/oss-fuzz#9828, where
it looks like we'll have to sort out getting a DEBUG build of LLVM.
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 24, 2024
@fanquake @PastaPastaPasta
Merge bitcoin#27447: depends: Remove _LIBCPP_DEBUG from depends DEB…
9b58b2d 
…UG mode

bc4fd49 depends: add _LIBCPP_ENABLE_ASSERTIONS to DEBUG mode (fanquake)
cf266b2 depends: Remove _LIBCPP_DEBUG from depends DEBUG mode (fanquake)

Pull request description:

  It was deprecated in LLVM 15, turned into compile-time error in LLVM 16:
  ```bash
  In file included from /usr/lib/llvm-16/bin/../include/c++/v1/cassert:19:
  /usr/lib/llvm-16/bin/../include/c++/v1/__assert:22:5: error: "Defining _LIBCPP_DEBUG is not supported anymore.
  Please use _LIBCPP_ENABLE_DEBUG_MODE instead."
      ^
  1 error generated.
  ```

  and has been removed entirely in LLVM 17 (main): llvm/llvm-project@ff573a4.

  [Building libc++ in debug mode](https://releases.llvm.org/16.0.0/projects/libcxx/docs/DesignDocs/DebugMode.html), will also automatically set
  `_LIBCPP_ENABLE_DEBUG_MODE` (the new define), so adding it to depends
  doesn't seem useful, and would just result in redefinition errors.

  I'm wondering if as a followup, we could enable a DEBUG build of libc++
  in our MSAN CI job? i.e https://github.com/fanquake/bitcoin/tree/msan_with_enable_debug_mode.

  Somewhat related to google/oss-fuzz#9828, where
  it looks like we'll have to sort out getting a DEBUG build of LLVM, and can drop the commentary about re-enabling DEBUG=1.

ACKs for top commit:
  MarcoFalke:
    lgtm Approach ACK bc4fd49

Tree-SHA512: 9c0f48fc428278fbf34fbb8f81e761e232506d7ab28e971cb9a9b9a81d549b4d8bbe51e2f7608d56e489428679231da5b7431443849b238a8a993ad241740282
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maflcko maflcko maflcko approved these changes

@dergoegge dergoegge dergoegge approved these changes

@jonathanmetzman jonathanmetzman jonathanmetzman approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@fanquake @jonathanmetzman @maflcko @dergoegge