Skip to content

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Apr 3, 2023

Add an option that when passed, will disable hardening options, and pass --disable-hardening through to configure. Due to the way we link libssp for Windows builds, they now fail (after #27118), if building with depends, and configuring with --disable-hardening (Windows is the odd build out here). See: #27118 (comment).

This change would add a depends option such that, if someone wants to build with depends, for Windows, without hardening, they can do so. This may also be useful when building for debugging.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 3, 2023

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

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK hebasto
Concept ACK theuni

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24123 (build: Pointer Authentication and Branch Target Identification for aarch64 Linux (Guix) 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
Copy link
Member Author

fanquake commented Apr 3, 2023

cc @hebasto @TheCharlatan

Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

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

ACK modulo spelling nit.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 24ef4bb, tested on Ubuntu 22.04.

Happy to re-ACK after fixing a typo :)

Add an option that when passed, will disable hardening options, and
pass `--disable-hardening` through to configure. Due to the way
we link libssp for Windows builds, they now fail (after bitcoin#27118),
if building with depends, and configuring with --disable-hardening.
See:
bitcoin#27118 (comment).

This change would add a depends opiton such that, if someone wants to
build with, for windows, without hardening, they can do so. This may
also be useful when building for debugging.
@fanquake fanquake force-pushed the depends_add_HARDEN branch from 24ef4bb to 436df1e Compare April 4, 2023 09:08
@fanquake
Copy link
Member Author

fanquake commented Apr 4, 2023

Pushed to fix the typo.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

re-ACK 436df1e

@fanquake fanquake merged commit 75d807a into bitcoin:master Apr 5, 2023
@fanquake fanquake deleted the depends_add_HARDEN branch April 5, 2023 11:30
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 5, 2023
436df1e depends: add NO_HARDEN option (fanquake)

Pull request description:

  Add an option that when passed, will disable hardening options, and pass `--disable-hardening` through to configure. Due to the way we link `libssp` for Windows builds, they now fail (after bitcoin#27118), if building with depends, and configuring with `--disable-hardening` (Windows is the odd build out here). See: bitcoin#27118 (comment).

  This change would add a depends option such that, if someone wants to build with depends, for Windows, without hardening, they can do so. This may also be useful when building for debugging.

ACKs for top commit:
  hebasto:
    re-ACK 436df1e

Tree-SHA512: 5a3ef5ec87b10a5ad0a284201988ce94789451735c7c7e20d337f7232955b0b9a0addab1c3b5725755f00d8ce6741aa9c8cb5e3d48d926515b7dde46acdbcaa0
Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

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

Post-merge ACK 436df1e

@bitcoin bitcoin locked and limited conversation to collaborators Apr 4, 2024
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.

4 participants