Skip to content

Conversation

fanquake
Copy link
Member

Setting these values has been redundant since autoconf ~2.60, and we
require 2.69. Should not change behaviour. Includes minor formatting
improvements.

secp256k1 also recently made the same change:
bitcoin-core/secp256k1#1079.

@fanquake
Copy link
Member Author

cc @real-or-random

@real-or-random
Copy link
Contributor

The automatic setting works only for variables enable_foo ( --enable-foo) or variables with_foo (--with-foo). But some of the removed assignments assign to use_foo or build_bitcoin_foo variables.

@fanquake
Copy link
Member Author

The automatic setting works only for variables enable_foo ( --enable-foo) or variables with_foo (--with-foo).

😅 Right. I guess we try should clean these up as well; given we currently use a mix of everything throughout configure.. Will update the changes.

@fanquake fanquake force-pushed the remove_uneeded_configure_defaults branch from 4ef6189 to 9c260d3 Compare March 30, 2022 20:26
@fanquake
Copy link
Member Author

Right. I guess we try should clean these up as well; given we currently use a mix of everything throughout configure.. Will update the changes.

Have done this now. The changes are too granular as-is, and can be split-out / squashed down, but this should better reflect what we want to accomplish.

@jb55
Copy link
Contributor

jb55 commented Mar 30, 2022 via email

@DrahtBot
Copy link
Contributor

DrahtBot commented Mar 31, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24958 (build: Fix macOS Apple M1 build with miniupnpc and libnatpmp. Again :) by hebasto)
  • #24798 ([POC] build: Hello Qt 6 by hebasto)
  • #24291 (build: Remove negated --enable-fuzz checks from build system by MarcoFalke)
  • #24051 (Bugfix: configure: bitcoin-{cli,tx,util} don't need UPnP, NAT-PMP, or ZMQ by luke-jr)
  • #22644 (Deprecate UPnP support, require 2.1 or later by fanquake)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@fanquake fanquake force-pushed the remove_uneeded_configure_defaults branch from 9c260d3 to 92c9d75 Compare April 20, 2022 12:48
@fanquake
Copy link
Member Author

Rebased past #24391 and #24681. I'll start splitting a few changes off from here so we can move things forward.

@DrahtBot DrahtBot mentioned this pull request Apr 20, 2022
5 tasks
Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

I guess it's clear from the context but:

Concept ACK

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@laanwj
Copy link
Member

laanwj commented Jun 2, 2022

Concept and light code review ACK 92c9d75

@fanquake
Copy link
Member Author

May follow up with some related changes later on.

@fanquake fanquake closed this Aug 31, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Aug 31, 2023
@fanquake fanquake deleted the remove_uneeded_configure_defaults branch September 14, 2023 10:33
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