-
Notifications
You must be signed in to change notification settings - Fork 36.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
build, refactor: Drop useless call
Make function
#24285
Conversation
Guix builds:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review ACK e644591
Since in all the instances of call()
addressed in this PR, $(1)
is equivalent to $(package)
, and for each package, there is a separate $(package)_*_cmds
, where each one had the value of there $(package)
, hence theoretically there is no need for $(1)
value for them.
I also cross-checked for all the $(package)_*_cmds
functions to see if any of them was using the $(package)
as the parameter provided from call()
, and I found none. So I think the changes applied in this PR are safe to merge.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
e644591 build, refactor: Drop useless `call` Make function (Hennadii Stepanov) Pull request description: Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all. This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about. No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/bitcoin#21995. ACKs for top commit: laanwj: Code review ACK e644591 shaavan: Code review ACK e644591 Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
…imestamps deterministic 6ebe576 build: Make dependency package archive timestamps deterministic (Hennadii Stepanov) Pull request description: This PR makes testing changes like bitcoin/bitcoin#20641, bitcoin/bitcoin#21593, bitcoin/bitcoin#22142, bitcoin/bitcoin#24279, bitcoin/bitcoin#24285 as easy as comparing hashes. With this PR: ``` $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-ca950bd6d13.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-b537c466dcb.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-e13b2bdd8b8.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-82255b84667.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-c43fc4cf2f6.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-663de0dc628.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-986926343e2.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bec6a4d3299.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-8ae81bab6f4.tar.gz ``` As an example, here is an evidence that bitcoin/bitcoin#24279 is a strict refactoring change: ``` $ git fetch origin pull/24279/head $ git cherry-pick 706026838d917a3d853e03e83db040f1fd4aeb74 $ git cherry-pick 3f90ddea8a6a2061cfb347a1d77df2c0a6fa238c $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-c7faf31d5ca.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-1af3dd1d99e.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-6228a9f8534.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-41aa6194ecc.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-6a93027769c.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-d40cb2d45c9.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-120c3cb745d.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bbd4d813c69.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-df0858a19d2.tar.gz ``` ACKs for top commit: TheCharlatan: Code review ACK 6ebe576 Tree-SHA512: 20e0222781f5dcb50126c11677d0671bcdd7be144b2e528c75a02983acc494206552fb35039697ccd094de27a21b3fb439e9965c34feb8a6d74627fa20a9a5e7
…s deterministic 6ebe576 build: Make dependency package archive timestamps deterministic (Hennadii Stepanov) Pull request description: This PR makes testing changes like bitcoin#20641, bitcoin#21593, bitcoin#22142, bitcoin#24279, bitcoin#24285 as easy as comparing hashes. With this PR: ``` $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-ca950bd6d13.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-b537c466dcb.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-e13b2bdd8b8.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-82255b84667.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-c43fc4cf2f6.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-663de0dc628.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-986926343e2.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bec6a4d3299.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-8ae81bab6f4.tar.gz ``` As an example, here is an evidence that bitcoin#24279 is a strict refactoring change: ``` $ git fetch origin pull/24279/head $ git cherry-pick 7060268 $ git cherry-pick 3f90dde $ make -C depends clean $ make -C depends HOST=x86_64-w64-mingw32 $ find depends/built/x86_64-w64-mingw32 -name '*.hash' | sort | xargs cat 1f685a61cbf205f81977ecf88cba91fa1ccdfbe77ab4ec3405dcd33ceb778af4 bdb-4.8.30-c7faf31d5ca.tar.gz 08a9acde276e6e5e5c8913e3ad07eeecda184a996882ae226b3ed056c7ec1b01 boost-1.80.0-1af3dd1d99e.tar.gz 144c6d92e4108fcc90740bee27007db58a88336a97be6367f9c8ba4cc208af27 libevent-2.1.12-stable-6228a9f8534.tar.gz e3c9c9609bf32bfd460432c6ab99a64e9f8750ed775a193925ff4f5aed363e4c libnatpmp-07004b97cf691774efebe70404cf22201e4d330d-41aa6194ecc.tar.gz 62c6a089a4b24a413eccd2f389bf4c8b0716423b0ace5e87e984069635da9f83 miniupnpc-2.2.2-6a93027769c.tar.gz 78762700066273e597698a78479a506b33532ea565d18ef561614b9fc3820cf5 qrencode-3.4.4-d40cb2d45c9.tar.gz 5e2183faf91838510a48e6dbb4b65ae74a7d48ba1abc070b82767c4076582360 qt-5.15.5-120c3cb745d.tar.gz 9f8459f8d27fc3af9146712be6ba6577f15741429936504a950cc51c17da1ba8 sqlite-3380500-bbd4d813c69.tar.gz 0eca5d01d427de50be4bd57c8bb100ab69b017792c32b8733e2b20443f4c9c28 zeromq-4.3.4-df0858a19d2.tar.gz ``` ACKs for top commit: TheCharlatan: Code review ACK 6ebe576 Tree-SHA512: 20e0222781f5dcb50126c11677d0671bcdd7be144b2e528c75a02983acc494206552fb35039697ccd094de27a21b3fb439e9965c34feb8a6d74627fa20a9a5e7
Using the
call
function with$(package)_*_cmds
is effectively noop because the latter, which could be found in<package>.mk
files, do not use temporary$(1)
variable at all.This PR removes useless calls of the
call
function, and makes code more readable and easier to reason about.No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/#21995.