Skip to content

Conversation

@dgoncharov
Copy link

The purpose of this commit is to reduce the amount of work make does.
This simplifies make -d output and improves performance (this
performance gain is hardly noticeable).

By default make remakes all makefiles.
This build system does not have rules to remake makefiles.
This commit adds an explicit rule for each makefile.

Each rule serves 2 purposes.

Each rule prevents make from searching for an implicit rule.
Once found, the rule prevents make from remaking the makefile.
That's why the rule is double colon. Make won't use a double colon rule to
remake a makefile. See
https://www.gnu.org/software/make/manual/make.html#Remaking-Makefiles.

$ # on this branch.
$ make -d print-host |grep expat
Reading makefile 'packages/expat.mk' (search path) (no ~ expansion)...
Makefile 'packages/expat.mk' might loop; not remaking it.
$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
$ make -d print-host |grep expat |wc
367 1916 19918
$

Make prints 367 lines per makefile.
With this change make prints 2 lines per makefile.

This changeset was originally submitted as pr #22237.
I am closing #22237 and resubmitting the same changeset here.

gtkiller added 2 commits June 13, 2021 16:22
The purpose of this commit is to reduce the amount of work make does.
This simplifies make -d output and improves performance (this
performance gain is hardly noticeable).

By default make remakes all makefiles.
This build system does not have rules to remake makefiles.
This commit adds an explicit rule for each makefile.

Each rule serves 2 purposes.
1. Each rule prevents make from searching for an implicit rule.
2. Once found, the rule prevents make from remaking the makefile.
That's why the rule is double colon. Make won't use a double colon rule to
remake a makefile. See
https://www.gnu.org/software/make/manual/make.html#Remaking-Makefiles.

$ # on this branch.
$ make -d print-host |grep expat
Reading makefile 'packages/expat.mk' (search path) (no ~ expansion)...
Makefile 'packages/expat.mk' might loop; not remaking it.
$ git co master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
$ make -d print-host |grep expat |wc
    367    1916   19918
$

Make prints 367 lines per makefile.
With this change make prints 2 lines per makefile.
* master: (436 commits)
  Test that descriptor wallet upgrade does nothing
  Make IsSegWitOutput return true for taproot outputs
  bench: fix 32-bit narrowing warning in bench/peer_eviction.cpp
  Change ScriptPubKeyMan::Upgrade to default to return true
  test: move rpc_rawtransaction tests to < 30s group
  test: whitelist rpc_rawtransaction peers to speed up tests
  test: Fix wallet_listdescriptors.py if bdb is not compiled
  Move implementations of non-template fuzz helpers
  refactor: move UpdateTip into CChainState
  refactor: no mempool arg to GetCoinsCacheSizeState
  refactor: move UpdateMempoolForReorg into CChainState
  validation: make CChainState::m_mempool optional
  init: remove straggling boost thread_group code
  fix incorrect testmempoolaccept doc
  doc: mention that we enforce port=0 in I2P
  addrman: reset I2P ports to 0 when loading from disk
  test: ensure I2P ports are handled as expected
  net: do not connect to I2P hosts on port!=0
  net: distinguish default port per network
  net: change I2P seeds' ports to 0
  ...
@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 19, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23603 (build: Fix x86_64 <-> arm64 cross-compiling for macOS by hebasto)
  • #22380 (build: add and use C_STANDARD and CXX_STANDARD in depends by fanquake)
  • #22283 (build: Replace $(AT) with .SILENT by dgoncharov)
  • #21778 (POC: LLVM 13 & 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
Copy link
Member

hebasto commented Aug 19, 2021

Concept ACK.

It seems this PR should be rebased on top of the recent master branch.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 9, 2021

🐙 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".

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 3, 2022

There hasn't been much activity lately and the patch still needs rebase. What is the status here?
  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@achow101
Copy link
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.

cc @dongcarl

@achow101 achow101 closed this Oct 12, 2022
@bitcoin bitcoin locked and limited conversation to collaborators Oct 12, 2023
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.

6 participants