Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Feb 8, 2022

No need to specify $(package)_download_file when it is equal to $(package)_file_name.

Historically, before #19817, distinct $(package)_download_file and $(package)_file_name were used for better portability (I guess) by removing + characters from a file name.

The only package which still use file renaming is native_capnp:

$(package)_download_path=https://capnproto.org/
$(package)_download_file=capnproto-c++-$($(package)_version).tar.gz
$(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz

No need to specify `$(package)_download_file` when it is equal to
`$(package)_file_name`.
Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

ACK d644c45

According to packages.md:

$(package)_download_file:
The file-name of the upstream source if it differs from how it should be
stored locally.
This can be used to avoid storing file-names with strange
characters.

So it makes sense to remove $(package)_download_file where it is identical to $(package)_file_name.

I verified that the changes done in this PR are correct and complete. I also confirmed that $(package)_download_file and $(package)_file_name are different for native_capnp.mk, and hence it cannot be touched under this PR.

@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 8, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #22552 (build: Improve depends build system robustness by hebasto)
  • #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.

Copy link
Member

@fanquake fanquake left a comment

Choose a reason for hiding this comment

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

ACK d644c45

Guix build (Darwin):

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
6b3b0b4b81b5b06d5bca94df20b3d18020f9f09c398e4a4859d2bf2c84a7520b  guix-build-d644c45e39c4/output/arm64-apple-darwin/SHA256SUMS.part
56605f3af7443f3d4689ff8bd40ea9dbb95b8272ba599a635c9f3a052e9cac9c  guix-build-d644c45e39c4/output/arm64-apple-darwin/bitcoin-d644c45e39c4-arm64-apple-darwin.tar.gz
5dfeddbf16f92b7b36a585f434c9eccbc0a7f3645942874ea15d88f15aced7d6  guix-build-d644c45e39c4/output/arm64-apple-darwin/bitcoin-d644c45e39c4-osx-unsigned.dmg
e1bcd610edf36a84204d96e5f3c2b608824fae0ab064936374555e67addbd47b  guix-build-d644c45e39c4/output/arm64-apple-darwin/bitcoin-d644c45e39c4-osx-unsigned.tar.gz
e78c83585c48861152f0fb68548d20721ea75b4e4b683660ca556d8fd5c84527  guix-build-d644c45e39c4/output/dist-archive/bitcoin-d644c45e39c4.tar.gz
ef3808586175dae3283aee9e35486c4cd2bee479999f5353702c5ee2f50c80a5  guix-build-d644c45e39c4/output/x86_64-apple-darwin/SHA256SUMS.part
dd9fd78b4956ff5ed9a66be518db2fc2c1ddc7ebfcd0eb7c7aa039dc96790dfb  guix-build-d644c45e39c4/output/x86_64-apple-darwin/bitcoin-d644c45e39c4-osx-unsigned.dmg
389d679abea6eca931a223e31d65474ce9e49cbcb045f5d646527f6c929d1cf9  guix-build-d644c45e39c4/output/x86_64-apple-darwin/bitcoin-d644c45e39c4-osx-unsigned.tar.gz
c565625529613da282a74c1bffccfa0e43512463fa2797e720de5d5aa9d56bc1  guix-build-d644c45e39c4/output/x86_64-apple-darwin/bitcoin-d644c45e39c4-osx64.tar.gz

@fanquake fanquake merged commit 6ac637f into bitcoin:master Feb 9, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 9, 2022
…load_file` assignments

d644c45 build, refactor: Drop redundant `$(package)_download_file` assignments (Hennadii Stepanov)

Pull request description:

  No need to specify `$(package)_download_file` when it is equal to `$(package)_file_name`.

  Historically, before bitcoin#19817, distinct `$(package)_download_file` and `$(package)_file_name` were used for better portability (I guess) by removing `+` characters from a file name.

  The only package which still use file renaming is `native_capnp`: https://github.com/bitcoin/bitcoin/blob/eca694a4e78d54ce4e29b388b3e81b06e55c2293/depends/packages/native_capnp.mk#L3-L5

ACKs for top commit:
  shaavan:
    ACK d644c45
  fanquake:
    ACK d644c45

Tree-SHA512: 488dd0f55cea077174e78a75d8385bacb1a5463883cadeb5fd7c9426865ea5f3a8bad0bd6e8e9d530bce6f0c1715349b3fbabb4e22634348cdd68f5fc8a3c53b
@hebasto hebasto deleted the 220208-filename branch February 9, 2022 18:53
@DrahtBot
Copy link
Contributor

Guix builds

File commit eca694a
(master)
commit f0658bb
(master and this pull)
SHA256SUMS.part 52bb278f472c04ff... a56240b36207f8ea...
*-aarch64-linux-gnu-debug.tar.gz 1ea67072871fb3e9... 85341a2680cd9fa6...
*-aarch64-linux-gnu.tar.gz ab7737ea08cce568... b98c03b15b211c53...
*-arm-linux-gnueabihf-debug.tar.gz ae6c042bb3938d92... 9091f8741ccfe179...
*-arm-linux-gnueabihf.tar.gz 13c2fd99fa3ff645... 7327c7cefa82c8d7...
*-arm64-apple-darwin.tar.gz f2af6d80043feda6... 4512f00fd0418e69...
*-osx-unsigned.dmg 77200c90800d39ed... cc083b7536598de7...
*-osx-unsigned.tar.gz 9f07da42c43106fb... dc259a6fb377af19...
*-osx64.tar.gz 43d3aee5c66221ba... 494de7ea82c1d952...
*-powerpc64-linux-gnu-debug.tar.gz 4ba7a36e05279e3b... ef2ae7bac81f61f8...
*-powerpc64-linux-gnu.tar.gz 882fe0c68aad339a... dd0b27478fd04858...
*-powerpc64le-linux-gnu-debug.tar.gz d35588fadf892dea... 59e915a7ec1ff4bb...
*-powerpc64le-linux-gnu.tar.gz 19808899c534b9b0... bc580cf758fdb2c8...
*-riscv64-linux-gnu-debug.tar.gz 1693a2d394dccb60... a81d30b9ad1840bd...
*-riscv64-linux-gnu.tar.gz 27cdc3197b74b915... d211fffd8dd5d5c8...
*-x86_64-linux-gnu-debug.tar.gz 0ea261146b90e7dc... d1f96ba8af30fb64...
*-x86_64-linux-gnu.tar.gz bca2953d8b7fe08a... 8f8ee5dd94679a5c...
*.tar.gz 527dddd72b8fb635... d643a6d54b2c9514...
guix_build.log 066fdaf5e045c1dc... 5264d8edcbc64571...
guix_build.log.diff a43b251d739f61fb...

PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 7, 2022
…load_file` assignments

d644c45 build, refactor: Drop redundant `$(package)_download_file` assignments (Hennadii Stepanov)

Pull request description:

  No need to specify `$(package)_download_file` when it is equal to `$(package)_file_name`.

  Historically, before bitcoin#19817, distinct `$(package)_download_file` and `$(package)_file_name` were used for better portability (I guess) by removing `+` characters from a file name.

  The only package which still use file renaming is `native_capnp`: https://github.com/bitcoin/bitcoin/blob/eca694a4e78d54ce4e29b388b3e81b06e55c2293/depends/packages/native_capnp.mk#L3-L5

ACKs for top commit:
  shaavan:
    ACK d644c45
  fanquake:
    ACK d644c45

Tree-SHA512: 488dd0f55cea077174e78a75d8385bacb1a5463883cadeb5fd7c9426865ea5f3a8bad0bd6e8e9d530bce6f0c1715349b3fbabb4e22634348cdd68f5fc8a3c53b
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Apr 11, 2022
…load_file` assignments

d644c45 build, refactor: Drop redundant `$(package)_download_file` assignments (Hennadii Stepanov)

Pull request description:

  No need to specify `$(package)_download_file` when it is equal to `$(package)_file_name`.

  Historically, before bitcoin#19817, distinct `$(package)_download_file` and `$(package)_file_name` were used for better portability (I guess) by removing `+` characters from a file name.

  The only package which still use file renaming is `native_capnp`: https://github.com/bitcoin/bitcoin/blob/eca694a4e78d54ce4e29b388b3e81b06e55c2293/depends/packages/native_capnp.mk#L3-L5

ACKs for top commit:
  shaavan:
    ACK d644c45
  fanquake:
    ACK d644c45

Tree-SHA512: 488dd0f55cea077174e78a75d8385bacb1a5463883cadeb5fd7c9426865ea5f3a8bad0bd6e8e9d530bce6f0c1715349b3fbabb4e22634348cdd68f5fc8a3c53b
gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 8, 2022
…load_file` assignments

d644c45 build, refactor: Drop redundant `$(package)_download_file` assignments (Hennadii Stepanov)

Pull request description:

  No need to specify `$(package)_download_file` when it is equal to `$(package)_file_name`.

  Historically, before bitcoin#19817, distinct `$(package)_download_file` and `$(package)_file_name` were used for better portability (I guess) by removing `+` characters from a file name.

  The only package which still use file renaming is `native_capnp`: https://github.com/bitcoin/bitcoin/blob/eca694a4e78d54ce4e29b388b3e81b06e55c2293/depends/packages/native_capnp.mk#L3-L5

ACKs for top commit:
  shaavan:
    ACK d644c45
  fanquake:
    ACK d644c45

Tree-SHA512: 488dd0f55cea077174e78a75d8385bacb1a5463883cadeb5fd7c9426865ea5f3a8bad0bd6e8e9d530bce6f0c1715349b3fbabb4e22634348cdd68f5fc8a3c53b
@bitcoin bitcoin locked and limited conversation to collaborators Feb 10, 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.

5 participants