DrahtBot
commented at 1:18 pm on July 26, 2021:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#25697 (depends: expat 2.4.8 & fix building with -flto by fanquake)
#25696 (build: Re-enable external signer on Windows by hebasto)
#25633 (depends: don’t restrict –enable-lto to non-guix cctools by fanquake)
#25391 (guix: Use LTO to build releases by fanquake)
#24283 (build: Add show-% target for multi-line variables and debug info by hebasto)
#24123 ([POC] build: enable Pointer Authentication and Branch Target Identification for aarch64 (Linux) by fanquake)
#21778 (build: LLVM 14 & LLD based macOS toolchain 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.
hebasto marked this as a draft
on Jul 26, 2021
hebasto force-pushed
on Jul 26, 2021
hebasto marked this as ready for review
on Jul 26, 2021
jarolrod
commented at 3:02 pm on July 27, 2021:
member
Concept ACK
I agree with the vision this PR is trying to set. I wanted to try to document what this seems to be doing and aims to accomplish.
What this PR is doing:
We can notice some “fragility” in our build system when looking at the recent Android related issues and their patches. The OP shows a good example; somehow, the boost version ended up there. Another example is the patch set of #22555; it seems as though the wrong value is evaluated with specific variables.
Why might this be happening? This PR hits the nail on the head by attacking the use of recursive variables. Recursive variables make it quite hard to track where the value is coming from (to a build noob like me). The main change this PR is performing is refactoring from recursively expanded variables to simple expanded ones. This irons out any kinks that currently exist and seems to work for the Android build issues as reported here: #22555 (comment)
“To avoid all the problems and inconveniences of recursively expanded variables, there is another flavor: simply expanded variables.”
Should we do this?
Warning: The following opinions are from a build noob
We do not need to refactor to simply expanded variables. We could spend time figuring out what about our current setup and the use of recursive variables is contributing to the example in the OP and partially leading to other examples like #22522.
Nevertheless, we should move to simply expanded variables. The clear benefit is, again, stated in the make manual:
“Simply expanded variables generally make complicated makefile programming more predictable because they work like variables in most programming languages.”
Performing this refactor makes future changes more accessible because we can easily track where values come from. Also, and while not a directly supporting reason for this change, it can lower the entry and learning curve barrier to contributing to our build system.
Also, simply expanded variables are newer. Newer = Better βΊοΈ
Notes on commits:
dc54e4ad2c58cff78ad618f1c577aae704bc7901
simlar to explanation for 2ee0cf35d09a4b7a08361ecf2576e0e15e93f5f2
c705d1ba86938387b9f4bb2e4b3eea6c8b2b44fe
This is mainly a cosmetic change. Not required
2ee0cf35d09a4b7a08361ecf2576e0e15e93f5f2
If we are going to refactor to simple variables, this is needed to keep the proper order of evaluation. X.mk depends on native_X.mk.
DrahtBot removed the label
DrahtBot Guix build requested
on Jul 28, 2021
DrahtBot added the label
Needs rebase
on Sep 2, 2021
hebasto force-pushed
on Sep 2, 2021
hebasto
commented at 9:01 am on September 2, 2021:
member
Rebased 638133188fa21f17e65e73f08c0ff7b87a58f446 -> ad2a195b616e1822cdd6d47c792cbf21a11df7c5 (pr22552.03 -> pr22552.04) due to the conflict with #22840.
DrahtBot removed the label
Needs rebase
on Sep 2, 2021
laanwj
commented at 12:57 pm on September 9, 2021:
member
Why might this be happening? This PR hits the nail on the head by attacking the use of recursive variables.
Thanks @jarolrod for your detailed explanation, it wasn’t kind of clear to me in the OP what was exactly fragile here, and how it is fixed. I don’t know a lot about makefile internals, but avoiding inadvertent issues that could result in wrong builds seems important. Concept ACK.
Would be good if @theuni has a look over the changes I think.
DrahtBot added the label
Needs rebase
on Nov 14, 2021
hebasto force-pushed
on Nov 15, 2021
hebasto
commented at 6:31 pm on November 15, 2021:
member
Rebased ad2a195b616e1822cdd6d47c792cbf21a11df7c5 -> f2812ac19667015423cdcea884745e8b79520dc7 (pr22552.04 -> pr22552.05) due to the conflict with #23494.
hebasto
commented at 6:36 pm on November 15, 2021:
member
DrahtBot removed the label
Needs rebase
on Nov 15, 2021
DrahtBot added the label
Needs rebase
on Nov 21, 2021
hebasto force-pushed
on Nov 21, 2021
hebasto
commented at 6:02 pm on November 21, 2021:
member
Rebased f2812ac19667015423cdcea884745e8b79520dc7 -> 61b03345460a6d249af7e8829455d20786764c7a (pr22552.05 -> pr22552.06) due to the conflict with #23535.
DrahtBot removed the label
Needs rebase
on Nov 21, 2021
DrahtBot added the label
Needs rebase
on Nov 25, 2021
hebasto force-pushed
on Nov 28, 2021
hebasto
commented at 9:13 am on November 28, 2021:
member
Rebased 61b03345460a6d249af7e8829455d20786764c7a -> 01938b9b32ee6915890ad0ae5f26a3d59310f82f (pr22552.06 -> pr22552.07) due to the conflict with #23580.
DrahtBot removed the label
Needs rebase
on Nov 28, 2021
DrahtBot added the label
Needs rebase
on Nov 30, 2021
hebasto force-pushed
on Nov 30, 2021
hebasto
commented at 1:38 pm on November 30, 2021:
member
Rebased 01938b9b32ee6915890ad0ae5f26a3d59310f82f -> a3a4c46f36cebe57f3eacfdf5335d5e766a8c86c (pr22552.07 -> pr22552.08) due to the conflict with #23618.
DrahtBot removed the label
Needs rebase
on Nov 30, 2021
DrahtBot added the label
Needs rebase
on Dec 3, 2021
hebasto force-pushed
on Dec 4, 2021
hebasto
commented at 6:08 pm on December 4, 2021:
member
Rebased a3a4c46f36cebe57f3eacfdf5335d5e766a8c86c -> 2c586571bc7c0877a4e76906cdd752cba1d9a2ec (pr22552.08 -> pr22552.09) due to the conflict with #23489.
DrahtBot removed the label
Needs rebase
on Dec 4, 2021
bitcoin deleted a comment
on Dec 5, 2021
maflcko added the label
DrahtBot Guix build requested
on Dec 5, 2021
DrahtBot added the label
Needs rebase
on Dec 9, 2021
fanquake referenced this in commit
65b49f60a4
on Dec 10, 2021
hebasto force-pushed
on Dec 10, 2021
hebasto
commented at 4:13 pm on December 10, 2021:
member
Rebased 2c586571bc7c0877a4e76906cdd752cba1d9a2ec -> fe44782aec88f59f75b1461904a1be1c7eaaa7bc (pr22552.09 -> pr22552.10) due to the conflicts with #23495 and #23673.
DrahtBot removed the label
Needs rebase
on Dec 10, 2021
sidhujag referenced this in commit
4890778588
on Dec 10, 2021
hebasto
commented at 2:25 pm on December 11, 2021:
member
DrahtBot removed the label
DrahtBot Guix build requested
on Dec 15, 2021
RandyMcMillan referenced this in commit
7f64ef8c81
on Dec 23, 2021
DrahtBot added the label
Needs rebase
on Jan 10, 2022
hebasto force-pushed
on Jan 10, 2022
hebasto
commented at 1:12 pm on January 10, 2022:
member
Rebased fe44782aec88f59f75b1461904a1be1c7eaaa7bc -> 77ec0f5ec7eff638b49d0915796a7ada53bf8ebd (pr22552.10 -> pr22552.11) due to the conflict with #23724.
DrahtBot removed the label
Needs rebase
on Jan 10, 2022
hebasto
commented at 5:40 pm on January 10, 2022:
member
DrahtBot added the label
Needs rebase
on Feb 9, 2022
hebasto force-pushed
on Feb 11, 2022
hebasto
commented at 1:11 pm on February 11, 2022:
member
Rebased 0931be3804208bc7bdca036dc15abaf47f6ea963 -> 17ef4c9c5888bdc2be18184f1fcdae7c0aac1eb8 (pr22552.12 -> pr22552.13) due to the conflict with #24288.
DrahtBot removed the label
Needs rebase
on Apr 21, 2022
DrahtBot added the label
Needs rebase
on May 4, 2022
hebasto force-pushed
on May 28, 2022
hebasto
commented at 11:55 am on May 28, 2022:
member
Rebased 4cba82dcb232ab2f46703d94511904c13c9445be -> 83f7c336d16e644848ca60bde11014164c1beb0d (pr22552.14 -> pr22552.15) due to the conflict with #25046.
DrahtBot removed the label
Needs rebase
on May 28, 2022
DrahtBot added the label
Needs rebase
on Jun 16, 2022
scripted-diff: Make (package)_X variables simply expanded ones
DrahtBot added the label
Needs rebase
on Jul 27, 2022
DrahtBot
commented at 1:30 pm on July 27, 2022:
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”.
achow101
commented at 6:35 pm on October 12, 2022:
member
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.
achow101 closed this
on Oct 12, 2022
fanquake referenced this in commit
3eaf7be6ad
on Dec 8, 2022
sidhujag referenced this in commit
5dde8cb2a6
on Dec 8, 2022
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2024-11-24 09:12 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me