Skip to content

Conversation

Sjors
Copy link
Member

@Sjors Sjors commented Jul 5, 2019

It seems that AC_DEFINE([HAVE_SYSTEM], [HAVE_STD__SYSTEM || HAVE_WSYSTEM] causes HAVE_SYSTEM to always be defined, so we need to use #if HAVE_SYSTEM instead of #if defined(HAVE_SYSTEM).

Followup for #15457, can be tested with #12557.

@qmma70
Copy link
Contributor

qmma70 commented Jul 6, 2019

Approach ACK. AC_DEFINE will always define the pre-processor symbol.

@fanquake fanquake requested a review from dongcarl July 6, 2019 02:01
Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

ACK 976b034.

@dongcarl
Copy link
Contributor

dongcarl commented Jul 6, 2019

ACK 976b034.

Perhaps (in another PR/issue) we should make sure that for other preprocessor symbols we mean define(FOOBAR) when we say it. @fanquake does that sound reasonable?

@fanquake fanquake changed the title [build]: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM) build: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM) Jul 7, 2019
Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK 976b034

Tested on my macOS system with master (584168c) and with #12557.

Perhaps (in another PR/issue) we should make sure that for other preprocessor symbols we mean define(FOOBAR) when we say it. @fanquake does that sound reasonable?

@dongcarl yes that sounds like a good idea.

@fanquake fanquake merged commit 976b034 into bitcoin:master Jul 7, 2019
fanquake added a commit that referenced this pull request Jul 7, 2019
976b034 [build]: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM) (Sjors Provoost)

Pull request description:

  It seems that `AC_DEFINE([HAVE_SYSTEM], [HAVE_STD__SYSTEM || HAVE_WSYSTEM]` causes `HAVE_SYSTEM` to always be defined, so we need to use `#if HAVE_SYSTEM` instead of `#if defined(HAVE_SYSTEM)`.

  Followup for #15457, can be tested with #12557.

ACKs for top commit:
  dongcarl:
    ACK 976b034.
  promag:
    ACK 976b034.
  fanquake:
    ACK 976b034

Tree-SHA512: b8cdd04c2ec399fd15638aef5d75ea0886ec1572d3cf4fcea27c193e1e6390344315908262cad8981a9b0a905ab9520619ce2ffe9a717f4ee6bfa8b028ebbdc6
@Sjors Sjors deleted the 2019/07/system-ifdef branch July 7, 2019 11:00
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Oct 22, 2021
…VE_SYSTEM)

976b034 [build]: use #if HAVE_SYSTEM instead of defined(HAVE_SYSTEM) (Sjors Provoost)

Pull request description:

  It seems that `AC_DEFINE([HAVE_SYSTEM], [HAVE_STD__SYSTEM || HAVE_WSYSTEM]` causes `HAVE_SYSTEM` to always be defined, so we need to use `#if HAVE_SYSTEM` instead of `#if defined(HAVE_SYSTEM)`.

  Followup for bitcoin#15457, can be tested with bitcoin#12557.

ACKs for top commit:
  dongcarl:
    ACK bitcoin@976b034.
  promag:
    ACK 976b034.
  fanquake:
    ACK 976b034

Tree-SHA512: b8cdd04c2ec399fd15638aef5d75ea0886ec1572d3cf4fcea27c193e1e6390344315908262cad8981a9b0a905ab9520619ce2ffe9a717f4ee6bfa8b028ebbdc6

# Conflicts:
#	src/init.cpp
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants