scripted-diff: Drop Darwin version for better maintainability #23585
Conversation
-BEGIN VERIFY SCRIPT- sed -i 's/darwin19/darwin/g' $(git grep --files-with-matches 'darwin19') -END VERIFY SCRIPT-
hebasto
force-pushed
the
211124-darwin
branch
from
bcfaf5f to
2f356a0
Compare
|
Does the Kernel version we build for not matter here? |
In terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system. In terms of the build system itself, the usage of the |
Guix builds: |
|
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. |
|
GUIX hashes, mine match @hebasto: |
|
I don't know anything about MacOS but this does make it more consistent with other operating systems, which also don't have kernel/OS version in the target tuple. |
|
ACK 2f356a0 |
If we just deleted all of our code, maintenance would be a lot easier too. I'd expect a PR like this to at least explain what it's deleting, why it might have been necessary (or maybe it never was), and why it's ok to delete now. Not just so that reviewers better understand what's going, but so that someone looking at this commit in the future has a better explanation for why something was deleted than "it'd make maintenance easier", which, when we have scripted diffs, is marginal reasoning. For an example of how a change like this isn't necessarily side-effect free; different versions of |
... yeah, and useless too.
See #23585 (comment) -- moved into the PR description.
Of course, darwin version can (and should) be considered for the |
|
Just finished building of What does the 42 mean here? If nothing, why maintain it? |
I don't know. However build, host and target all default to the result of config.guess, unless otherwise specified. My point is that I don't like assuming changes like this are side-effect free.
We don't pass
It does mean something, just not to us, right now, in the context of our build system. Other projects, such as LLVM, can, and do use the kernel version to make build decision. After looking at this some more I do think we can make this change. I'll review properly shortly. |
There was a problem hiding this comment.
ACK 2f356a0
Guix Build:
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
280757a217f78b3c7af2321b90f92f52c48edfcb19bb20ebb108ec9e3ce3664f guix-build-2f356a0ca8b0/output/aarch64-linux-gnu/SHA256SUMS.part
3341d29a2368ad872f9b387c1dc1d6ef897e151939d029a47dc61c482497b1ac guix-build-2f356a0ca8b0/output/aarch64-linux-gnu/bitcoin-2f356a0ca8b0-aarch64-linux-gnu-debug.tar.gz
51ae211cea091d49c08a914aa1c080d16f84319834561156495035455b6edc08 guix-build-2f356a0ca8b0/output/aarch64-linux-gnu/bitcoin-2f356a0ca8b0-aarch64-linux-gnu.tar.gz
cc91ff5608b826fab19e0debc4d32604713a7fe09ea128e882f749486af51e66 guix-build-2f356a0ca8b0/output/arm-linux-gnueabihf/SHA256SUMS.part
6e9697b64df8cbbf7f3ce28410d3b7150ed8f4a0ffb9e3bdb62a8eb2e4b0e8ae guix-build-2f356a0ca8b0/output/arm-linux-gnueabihf/bitcoin-2f356a0ca8b0-arm-linux-gnueabihf-debug.tar.gz
8c9069ac54e3469d66771b8b7718986384eb6baa7efd9905e1d4929aaec96de5 guix-build-2f356a0ca8b0/output/arm-linux-gnueabihf/bitcoin-2f356a0ca8b0-arm-linux-gnueabihf.tar.gz
5ab293b20fc4b2a2c36ed9ac21958fda71cebb1fce1eae523684015cfbe5b6da guix-build-2f356a0ca8b0/output/dist-archive/bitcoin-2f356a0ca8b0.tar.gz
a351e197717e1099f141c3538377e730f3c5ea8b3dea114f7e62ce4c4b779858 guix-build-2f356a0ca8b0/output/powerpc64-linux-gnu/SHA256SUMS.part
3163f4ab0d33929525b72ce15b2a03c12a3231b3bed22d24d2d4688aa72981f7 guix-build-2f356a0ca8b0/output/powerpc64-linux-gnu/bitcoin-2f356a0ca8b0-powerpc64-linux-gnu-debug.tar.gz
1e326f6115fdb51e3fb1d6433101bbdf7f5c2e259d4b372f224a913d6e4eb276 guix-build-2f356a0ca8b0/output/powerpc64-linux-gnu/bitcoin-2f356a0ca8b0-powerpc64-linux-gnu.tar.gz
1307b3b00a151a15baf4d2f57dece7c90750219059e33392f0bd3706a2793777 guix-build-2f356a0ca8b0/output/powerpc64le-linux-gnu/SHA256SUMS.part
a3fa92410e2d54bfd854bc90e6295028f63b31123ec8ae658e238f42e3532ba0 guix-build-2f356a0ca8b0/output/powerpc64le-linux-gnu/bitcoin-2f356a0ca8b0-powerpc64le-linux-gnu-debug.tar.gz
1a6d8a7c360dbf3e9b6ced4416f60950223f77115b3ac8e18007800ae7a11c41 guix-build-2f356a0ca8b0/output/powerpc64le-linux-gnu/bitcoin-2f356a0ca8b0-powerpc64le-linux-gnu.tar.gz
550cb5f74de68cce086084eab0a188fa982dcd63206de886d33ea4100f53447e guix-build-2f356a0ca8b0/output/riscv64-linux-gnu/SHA256SUMS.part
229a1fdc9f2dff173f5b1e5de148dd6e8c226876fda7a7392c7c5e04e0f6f650 guix-build-2f356a0ca8b0/output/riscv64-linux-gnu/bitcoin-2f356a0ca8b0-riscv64-linux-gnu-debug.tar.gz
b3cd671e12185223d01596de85af766fb01ad15035c0d2b7170daa05b744e4ef guix-build-2f356a0ca8b0/output/riscv64-linux-gnu/bitcoin-2f356a0ca8b0-riscv64-linux-gnu.tar.gz
3daa4bda0a8f6458620a21a45c8424d824f4a3b8ab1b52720811a94244c0903c guix-build-2f356a0ca8b0/output/x86_64-apple-darwin/SHA256SUMS.part
752bff4a2b9e064fbd22fa7a1f5e856191b01993dcfecdbb9d6c6abe02d291a6 guix-build-2f356a0ca8b0/output/x86_64-apple-darwin/bitcoin-2f356a0ca8b0-osx-unsigned.dmg
71a91c48671f577b90e64932cfecf74c81ef899a7a4640a6b82e8cb060bf1962 guix-build-2f356a0ca8b0/output/x86_64-apple-darwin/bitcoin-2f356a0ca8b0-osx-unsigned.tar.gz
1ebb78a700bef9920810ec66c9d8a836720306acfe93d8b4d5632dbbdd97189a guix-build-2f356a0ca8b0/output/x86_64-apple-darwin/bitcoin-2f356a0ca8b0-osx64.tar.gz
87bdc2694e2c412deb3cb716063a4fce74038fe966cbeb0c453ef829360924c4 guix-build-2f356a0ca8b0/output/x86_64-linux-gnu/SHA256SUMS.part
28145437ae8b242632724e212228e1d0439763caad15682ff2857ceeb295df45 guix-build-2f356a0ca8b0/output/x86_64-linux-gnu/bitcoin-2f356a0ca8b0-x86_64-linux-gnu-debug.tar.gz
1e5499a7877f4e51e0de6de0f79b7f58af35a880eaddbbd46727c6e2f6d50b3c guix-build-2f356a0ca8b0/output/x86_64-linux-gnu/bitcoin-2f356a0ca8b0-x86_64-linux-gnu.tar.gz
fe6800a2917953d043d828ae6b2115cec88d3026b2b5822eb04a14a57749d85e guix-build-2f356a0ca8b0/output/x86_64-w64-mingw32/SHA256SUMS.part
c012a84a6c86f49e5401836221b83ea638467763f4a8a24cfb591f3ee630ae3c guix-build-2f356a0ca8b0/output/x86_64-w64-mingw32/bitcoin-2f356a0ca8b0-win-unsigned.tar.gz
f0e56ab4aa8e80b2ff4c518c64902a2c94f9e4127d15f497037db767955e2485 guix-build-2f356a0ca8b0/output/x86_64-w64-mingw32/bitcoin-2f356a0ca8b0-win64-debug.zip
43a2a3c4689c9390b7790bd6ec39a4f6b839f86829511a3ad472b559ec90bd2e guix-build-2f356a0ca8b0/output/x86_64-w64-mingw32/bitcoin-2f356a0ca8b0-win64-setup-unsigned.exe
d31d0f042c196973597d8acb800f27456ffd5f41a9c3f5dae4ee028462277b67 guix-build-2f356a0ca8b0/output/x86_64-w64-mingw32/bitcoin-2f356a0ca8b0-win64.zip
…intainability 2f356a0 scripted-diff: Drop Darwin version for better maintainability (Hennadii Stepanov) Pull request description: After this PR, any macOS tools version bumping in the future will touch fewer files in the repo. Pointing a Darwin version for the `--host` system does not matter for the following reasons: - in terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system. - in terms of the build system itself, the usage of the `$(host)` variable is self-consistent enough. Btw `$(host_os)` value already has the version dropped: ``` $ make -C depends --no-print-directory print-host_os HOST=x86_64-apple-darwin19 host_os=darwin ``` ACKs for top commit: gruve-p: ACK bitcoin@2f356a0 promag: ACK 2f356a0. fanquake: ACK 2f356a0 Tree-SHA512: 374896ab0ba02b0d8b4b21431fe963bd213b0d09586e0898c13a4c5fa294c1b693f1b2c92880c245c4157c14217b4825b36522f461930477f4d2a727086ebb2a
…etter maintainability fc1d1df scripted-diff: Drop Darwin version for better maintainability (Hennadii Stepanov) Pull request description: After this PR, any macOS tools version bumping in the future will touch fewer files in the repo. Pointing a Darwin version for the `--host` system does not matter for the following reasons: - in terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system. - in terms of the build system itself, the usage of the `$(host)` variable is self-consistent enough. Btw `$(host_os)` value already has the version dropped: ``` $ make -C depends --no-print-directory print-host_os HOST=x86_64-apple-darwin19 host_os=darwin ``` ACKs for top commit: gruve-p: ACK bitcoin/bitcoin@fc1d1df promag: ACK fc1d1df. fanquake: ACK fc1d1df Tree-SHA512: 374896ab0ba02b0d8b4b21431fe963bd213b0d09586e0898c13a4c5fa294c1b693f1b2c92880c245c4157c14217b4825b36522f461930477f4d2a727086ebb2a
…intainability 2f356a0 scripted-diff: Drop Darwin version for better maintainability (Hennadii Stepanov) Pull request description: After this PR, any macOS tools version bumping in the future will touch fewer files in the repo. Pointing a Darwin version for the `--host` system does not matter for the following reasons: - in terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system. - in terms of the build system itself, the usage of the `$(host)` variable is self-consistent enough. Btw `$(host_os)` value already has the version dropped: ``` $ make -C depends --no-print-directory print-host_os HOST=x86_64-apple-darwin19 host_os=darwin ``` ACKs for top commit: gruve-p: ACK bitcoin@2f356a0 promag: ACK 2f356a0. fanquake: ACK 2f356a0 Tree-SHA512: 374896ab0ba02b0d8b4b21431fe963bd213b0d09586e0898c13a4c5fa294c1b693f1b2c92880c245c4157c14217b4825b36522f461930477f4d2a727086ebb2a
After this PR, any macOS tools version bumping in the future will touch fewer files in the repo.
Pointing a Darwin version for the
--hostsystem does not matter for the following reasons:in terms of the resulted binaries, we should only care about the minimum supported macOS version which is a separated parameter in our build system.
in terms of the build system itself, the usage of the
$(host)variable is self-consistent enough. Btw$(host_os)value already has the version dropped: