build: target aarch64 over armv7, add rudimentary support for Rust components in build system, update to macOS SDK 15.2 (from Xcode 16.0)#7109
Conversation
|
kwvg
changed the title
kwvg
changed the title
|
This pull request has conflicts, please rebase. |
, bitcoin#31099, bitcoin#31172, bitcoin#31450, bitcoin#31608, bitcoin#31818, bitcoin#29881, bitcoin#32458, bitcoin#32400, bitcoin#32760, bitcoin#33178, bitcoin#33312, bitcoin#33780, bitcoin#33181, bitcoin#34102, partial bitcoin#29023, bitcoin#30454, bitcoin#30509, bitcoin#30510, bitcoin#31105, bitcoin#32922, bitcoin#33489, bitcoin#33445 (build backports: part 5) ad5c299 doc: add release notes (Kittywhiskers Van Gogh) 6350e93 merge bitcoin#34102: capnp 1.3.0 (Kittywhiskers Van Gogh) e6e4ad0 merge bitcoin#33181: build for Linux HOSTS with `-static-libgcc` (Kittywhiskers Van Gogh) e403bba merge bitcoin#33780: disable libsanitizer in Linux GCC build (Kittywhiskers Van Gogh) 88108ac partial bitcoin#33445: Update Clang in "tidy" job (Kittywhiskers Van Gogh) f9686bb partial bitcoin#33489: Drop support for EOL macOS 13 (Kittywhiskers Van Gogh) 4a6de56 merge bitcoin#33312: Disable `UndefinedBinaryOperatorResult` check in `src/ipc` (Kittywhiskers Van Gogh) 00f46c2 merge bitcoin#33178: increase maximum allowed (runtime) GCC to 7 (Kittywhiskers Van Gogh) 49f815d partial bitcoin#32922: use notarized v28.2 binaries and fix macOS detection (Kittywhiskers Van Gogh) 7bf7523 merge bitcoin#32760: capnp 1.2.0 (Kittywhiskers Van Gogh) 369e875 merge bitcoin#32400: Use modern Windows randomness functions (Kittywhiskers Van Gogh) 62cb45b merge bitcoin#32458: move `*-check.py` scripts under `contrib/guix/` (Kittywhiskers Van Gogh) 362013a merge bitcoin#29881: use GCC 13 to build releases (Kittywhiskers Van Gogh) bfcf58a merge bitcoin#31818: remove test-security/symbol-check scripts (Kittywhiskers Van Gogh) 973eca0 merge bitcoin#31608: Clarify min macOS and Xcode version (Kittywhiskers Van Gogh) 7f80572 merge bitcoin#31450: disable gcov in base-linux-gcc (Kittywhiskers Van Gogh) e189624 merge bitcoin#31172: increase minimum supported Windows to 10.0 (Kittywhiskers Van Gogh) 8494bd6 partial bitcoin#31105: Update libmultiprocess library (Kittywhiskers Van Gogh) 29c8bd9 merge bitcoin#31099: drop macOS LLVM install instructions (Kittywhiskers Van Gogh) 84b1e17 fix: bump `darwin_cmake_system_version` to match `OSX_MIN_VERSION` (Kittywhiskers Van Gogh) cdc411d merge bitcoin#31048: Bump minimum supported macOS to 13.0 (Kittywhiskers Van Gogh) 5515a69 partial bitcoin#30510: Add IPC wrapper for Mining interface (Kittywhiskers Van Gogh) ce6504e merge bitcoin#27038: test for `_FORTIFY_SOURCE` usage in release binaries (Kittywhiskers Van Gogh) 140848d partial bitcoin#30509: Add -ipcbind option to bitcoin-node (Kittywhiskers Van Gogh) 9df31ce partial bitcoin#30454: Introduce CMake-based build system (Kittywhiskers Van Gogh) 310a652 merge bitcoin#30423: simplify `test-security-check` (Kittywhiskers Van Gogh) 3bb0e30 partial bitcoin#29023: add historical release notes for 26.0 (Kittywhiskers Van Gogh) c725a30 docs: add omitted "Compatibility" heading to release notes template (Kittywhiskers Van Gogh) Pull request description: ## Motivation While working on adding support for Rust to our build system (see [dash#7109](#7109)), there were sources of conflict between our current setup and the minimums demanded by Rust and our FFI crate, [`cxx`](https://github.com/dtolnay/cxx). Specifically, Dash Core currently targets building for Windows 7 ([source](https://github.com/dashpay/dash/blob/cc50446936f8436d9ac3a2442bb348d93d8ba314/configure.ac#L818), [`0x601`](https://learn.microsoft.com/en-gb/windows/win32/winprog/using-the-windows-headers)) in direct conflict with the decision by Rusts' maintainers to drop support for Windows 7 effective Rust 1.78 ([source](https://blog.rust-lang.org/2024/02/26/Windows-7/)) More recent versions of Rust have issues that require additional workarounds (see [`rust-lang/rust#128218`](rust-lang/rust#128218)) to keep support for Windows 7 around and using older versions of Rust is infeasible since both [`cxx`](https://github.com/dtolnay/cxx) and Rust packages currently under evaluation for integration with Dash Core require Rust 1.82+. Additionally, non-conformant definitions in the macOS SDK cause issues when attempting to build crates with FFI definitions (see [`dtolnay/cxx#1574`](dtolnay/cxx#1574)) which is resolved by an additional SDK bump that is not in the scope of this PR as it is _above_ the SDK version used upstream but does give cause to also consider a macOS target version bump, which _is_ done by upstream. ## Additional Information * Dependent on #6919 * Dependency for #7109 * macOS 11 (Big Sur) had its support period elapse ~2 years ago ([source](https://endoflife.date/macos)). macOS 14 (Sonoma) is the lowest version of macOS still receiving support from Apple and the new minimum version elected by upstream. * Windows 7 had its _extended_ support period elapse ~5 years ago ([source](https://learn.microsoft.com/en-gb/lifecycle/products/windows-7)) with the second lowest version, Windows 8.1, had its _extended_ support period elapse ~2 years ago ([source](https://learn.microsoft.com/en-us/lifecycle/products/windows-81)). ## Breaking Changes Dash Core binaries will now target Windows 10 and macOS 14 (Sonoma), replacing the previous target Windows 7 and macOS 11 (Big Sur). ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK ad5c299 Tree-SHA512: 650cbc12f2f129770623fd4e62bd7eb77ba0677ae33b765cc8359878633562e2a5c302f86d399400bb798d6a0ac5e6c030f3b8c657b63178f468bbe0d1b246a5
|
This pull request has conflicts, please rebase. |
Since the container image could be run on different architectures we can't implicitly assume that the x86_64 compiler is available, it needs to be made explicit. Additionally, we stopped releasing ARMv7 (32-bit) binaries a long time ago, we should test against ARMv8 (64-bit) instead.
kwvg
force-pushed
the
rust_build
branch
2 times, most recently
from
68f1157 to
9da49be
Compare
|
Guix Automation has began to build this PR tagged as v23.0.2-devpr7109.9da49bec. A new comment will be made when the image is pushed. |
Checksums for 9da49be |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.2-devpr7109.9da49bec. The image should be on dockerhub soon. |
|
Guix Automation has began to build this PR tagged as v23.0.2-devpr7109.5443107c. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.2-devpr7109.5443107c. The image should be on dockerhub soon. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.2-devpr7109.8bc0b2fa. The image should be on dockerhub soon. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@contrib/devtools/update-native-cxxbridge.py`:
- Around line 104-106: The tarfile extraction uses tar.extractall(tarball_path,
"r:gz") which is vulnerable to path traversal; replace the direct
tar.extractall(tmp_path) call with a safe extraction routine: iterate
tar.getmembers(), compute each member's destination by joining tmp_path and
member.name, check that
os.path.realpath(dest).startswith(os.path.realpath(tmp_path)) and raise on
violations, then extract only after validation (or use tar.extract(member,
tmp_path) for each safe member). Update the block around tarfile.open(...) and
use variables tar, tarball_path, and tmp_path referenced in the diff.
In `@contrib/guix/symbol-check.py`:
- Around line 151-159: The PE_ALLOWED_LIBRARIES set has mis-indented
continuation lines causing flake8 E122; fix by aligning each DLL entry (e.g.,
'ADVAPI32.dll', 'api-ms-win-core-synch-l1-2-0.dll', 'bcrypt.dll',
'IPHLPAPI.DLL', 'KERNEL32.dll', 'msvcrt.dll', 'ntdll.dll', 'SHELL32.dll') to the
same indentation level as the existing dict entries inside PE_ALLOWED_LIBRARIES
so the continuations line up with the opening brace and other entries,
preserving the trailing commas and existing comments.
🧹 Nitpick comments (1)
contrib/devtools/update-native-cxxbridge.py (1)
117-123: Consider adding a timeout to subprocess invocation.The
cargo checkcommand runs without a timeout, which could cause the script to hang indefinitely if there are network issues or dependency resolution problems.💡 Suggested improvement
# Run 'cargo check' print("Running cargo check --release --package=cxxbridge-cmd --bin=cxxbridge") result = subprocess.run( ["cargo", "check", "--release", "--package=cxxbridge-cmd", "--bin=cxxbridge"], cwd=cxx_dir, + timeout=300, # 5 minute timeout )
| PE_ALLOWED_LIBRARIES = { | ||
| 'ADVAPI32.dll', # legacy security & registry | ||
| 'api-ms-win-core-synch-l1-2-0.dll', # sync primitives (API set) | ||
| 'bcrypt.dll', # newer security and identity API | ||
| 'IPHLPAPI.DLL', # IP helper API | ||
| 'KERNEL32.dll', # win32 base APIs | ||
| 'msvcrt.dll', # C standard library for MSVC | ||
| 'ntdll.dll', # NT kernel API | ||
| 'SHELL32.dll', # shell API |
There was a problem hiding this comment.
Fix flake8 E122 indentation for new DLL entries.
Static analysis flags Line 153–158 for continuation indentation; please align these new lines with the surrounding dict entries.
🛠️ Proposed fix
- 'api-ms-win-core-synch-l1-2-0.dll', # sync primitives (API set)
+ 'api-ms-win-core-synch-l1-2-0.dll', # sync primitives (API set)
...
- 'ntdll.dll', # NT kernel API
+ 'ntdll.dll', # NT kernel API🧰 Tools
🪛 Flake8 (7.3.0)
[error] 152-152: continuation line missing indentation or outdented
(E122)
[error] 153-153: continuation line missing indentation or outdented
(E122)
[error] 154-154: continuation line missing indentation or outdented
(E122)
[error] 155-155: continuation line missing indentation or outdented
(E122)
[error] 156-156: continuation line missing indentation or outdented
(E122)
[error] 157-157: continuation line missing indentation or outdented
(E122)
[error] 158-158: continuation line missing indentation or outdented
(E122)
[error] 159-159: continuation line missing indentation or outdented
(E122)
🤖 Prompt for AI Agents
In `@contrib/guix/symbol-check.py` around lines 151 - 159, The
PE_ALLOWED_LIBRARIES set has mis-indented continuation lines causing flake8
E122; fix by aligning each DLL entry (e.g., 'ADVAPI32.dll',
'api-ms-win-core-synch-l1-2-0.dll', 'bcrypt.dll', 'IPHLPAPI.DLL',
'KERNEL32.dll', 'msvcrt.dll', 'ntdll.dll', 'SHELL32.dll') to the same
indentation level as the existing dict entries inside PE_ALLOWED_LIBRARIES so
the continuations line up with the opening brace and other entries, preserving
the trailing commas and existing comments.
Checksums for 1b56493 |
|
Guix Automation has began to build this PR tagged as v23.0.2-devpr7109.1b564935. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.2-devpr7109.1b564935. The image should be on dockerhub soon. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@configure.ac`:
- Around line 155-165: The AC_MSG_ERROR text refers to the wrong option
spelling; update the error string in the AC_MSG_ERROR call so it uses the
hyphenated option name --enable-online-rust (matching AC_ARG_ENABLE and the
ENABLE_ONLINE_RUST conditional) and keep the rest of the message unchanged;
locate the AC_MSG_ERROR that runs when enable_online_rust is "no" and
RUST_VENDORED_SOURCES is empty and replace `--enable_online_rust` with
`--enable-online-rust`.
In `@contrib/devtools/update-cargo-lock.py`:
- Around line 37-48: The current except only catches
subprocess.CalledProcessError so if subprocess.check_call raises OSError the
cargo_lock_bak won't be restored; change the except clause that currently reads
"except subprocess.CalledProcessError:" to catch all exceptions (e.g., "except
Exception:") so that when subprocess.check_call (or any other error) fails you
move cargo_lock_bak back to CARGO_LOCK and then re-raise; keep the existing
finally block that restores CARGO_CONFIG from CARGO_CONFIG_BAK and preserve the
cleanup logic that removes cargo_lock_bak if it exists.
In `@depends/funcs.mk`:
- Around line 340-350: The tar extraction in the int_cargo_preprocess_ext macro
uses $(build_TAR) -P -xf without --no-same-owner which can preserve ownership
when run as root; update the extraction invocation inside
int_cargo_preprocess_ext (the line invoking $(build_TAR)) to include the
--no-same-owner flag so it matches other extractions and avoids
permission/ownership issues when extracting vendored crates.
- Around line 308-314: The download_rust_std_target macro currently writes the
stamp file unconditionally; update it so that after downloading the
rust-std-$(rust_stdlib_version)-$(1).tar.gz you run the SHA256 verification (use
the existing build_SHA256SUM -c or equivalent to check
"$(SOURCES_PATH)/rust-std-$(rust_stdlib_version)-$(1).tar.gz" against
$(rust_stdlib_sha256_hash_$(1))) and only create the download-stamp file when
that verification succeeds; ensure the verification step uses the same
rust_stdlib_sha256_hash_$(1) token and aborts/does not write the stamp on
failure (place this check immediately before the echo to
"$(SOURCES_PATH)/download-stamps/.stamp_fetched-rust_stdlib-$(rust_stdlib_version)-$(rust_stdlib_sha256_hash_$(1)).hash").
🧹 Nitpick comments (3)
contrib/devtools/update-rust-hashes.py (2)
53-69: Shared helper functions are duplicated across both devtools scripts.
get_version,download_and_hash,write_stamp, andupdate_hash_in_fileare identical in bothupdate-rust-hashes.pyandupdate-native-cxxbridge.py. Consider extracting them into a shared module (e.g.,contrib/devtools/rust_helpers.py) to avoid drift.
62-69: No timeout onurllib.request.urlopen— the script can hang indefinitely on network issues.As a developer tool this is low-risk, but adding a
timeoutparameter would improve usability.💡 Suggested improvement
- with urllib.request.urlopen(url) as response: + with urllib.request.urlopen(url, timeout=60) as response:depends/funcs.mk (1)
316-321:download_rust_src_packageskips hash verification and stamp creation.Unlike
download_rust_std_target, this helper doesn't verify the hash or create a stamp file. This means re-running the download target won't skip already-fetched files unless the filename already exists on disk — and a corrupted file won't be detected until build time. This is acceptable for a convenience target but worth noting as a gap compared to the standard fetch path.
We're using `depends` to provide us with the Rust compiler and `cxxbridge`, we can drop it from the CI container and save some space.
The conflict occurs due a difference in glibc version between the Guix environment and the targets compiled, this is only a problem for Linux targets so the issue doesn't extend to Windows
macOS cross builds need `C{,PLUS}_INCLUDE_PATH` for Qt's native tooling
but causes issues when there are conflicting definitions between the
native and cross toolchains
This is an issue with more recent SDKs (observed from Xcode 15.3 15E204a shipped SDKs onwards) and explicit inclusion resolves it. We have to use `:=` to force immediate resolution.
Checksums for 409e205 |
|
Guix Automation has began to build this PR tagged as v23.0.2-devpr7109.409e205a. A new comment will be made when the image is pushed. |
|
Guix Automation has completed; a release should be present here: https://github.com/dashpay/dash-dev-branches/releases/tag/v23.0.2-devpr7109.409e205a. The image should be on dockerhub soon. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@configure.ac`:
- Around line 131-140: The RS_REQUIRE_PROG macro has unsafe quoting in the test
and uses double quotes for AC_MSG_ERROR; update the macro (around
RS_REQUIRE_PROG / AC_PATH_PROG usage) so the comparison is quoted as if test
"x${$1}" = "x${2_notfound}" (i.e., add surrounding double quotes for both sides)
and change the AC_MSG_ERROR argument to use m4 square brackets (e.g.,
AC_MSG_ERROR([Required program $2 was not found])) to avoid m4 quoting issues
and handle paths with spaces safely.
In `@contrib/devtools/update-rust-hashes.py`:
- Around line 53-69: The download_and_hash function currently treats any
existing dest file as complete; instead perform downloads into a temporary file
and atomically move it into place only on successful completion: open a temp
path (e.g., dest.with_suffix(".part") or use tempfile) when fetching via
urllib.request.urlopen, write and hash to that temp, then os.replace(temp, dest)
after the loop so partial downloads never overwrite dest; ensure existing dest
is still hashed when present and that the temp file is removed/cleaned up on
exceptions to avoid leaving junk.
In `@contrib/guix/symbol-check.py`:
- Around line 244-247: The PR title, the check_MACHO_sdk function, and the build
config are inconsistent: either update the SDK in the build system or change the
check; specifically, decide whether OSX_SDK_VERSION should be 15.2 and if so
update the build config (OSX_SDK_VERSION) to 15.2 and change check_MACHO_sdk to
compare to [15, 2, 0]; otherwise, change the PR title to say 15.0 (and/or change
check_MACHO_sdk to compare to [15, 0, 0] if you intended 15.0). Ensure
consistency between the build variable OSX_SDK_VERSION, the comparison in
check_MACHO_sdk, and the PR title.
🧹 Nitpick comments (5)
contrib/guix/libexec/fix-elf-interpreter.sh (2)
81-89: Consider checkingpatchelfexit status in the patching loop.If
patchelf --set-interpreteror--set-rpathfails on a binary, the loop silently continues. Since the script lacksset -e, a failed patch won't halt execution — potentially producing binaries with incorrect interpreters/RPATHs that pass through the build undetected. Adding|| exit 1(orset -eat the top) would surface such failures.Proposed fix
for binary in "${@}" do if [ -f "${binary}" ]; then echo "${SH_NAME}: patching ${binary}"; - "${SH_PATCHELF}" --set-interpreter "${GUIX_INTERP}" "${binary}"; - "${SH_PATCHELF}" --print-interpreter "${binary}" - "${SH_PATCHELF}" --set-rpath "${GUIX_RPATH}" "${binary}"; - "${SH_PATCHELF}" --print-rpath "${binary}" + "${SH_PATCHELF}" --set-interpreter "${GUIX_INTERP}" "${binary}" || exit 1; + "${SH_PATCHELF}" --print-interpreter "${binary}" + "${SH_PATCHELF}" --set-rpath "${GUIX_RPATH}" "${binary}" || exit 1; + "${SH_PATCHELF}" --print-rpath "${binary}" fi done
1-12: Guard against zero-argument invocation producing a spurious shift error.When invoked with no arguments,
shifton line 12 runs with$#=0, emitting a shell error before the usage check on line 23 catches it. A minor guard avoids the noise:LIBDIR="${1}" -shift +shift || trueThis is cosmetic since line 23 handles the case correctly.
contrib/devtools/update-native-cxxbridge.py (1)
18-57: Consider extracting shared helpers into a common module.
get_version,download_and_hash,write_stamp, andupdate_hash_in_fileare identical copies of those inupdate-rust-hashes.py. A sharedcontrib/devtools/rust_helpers.pymodule would eliminate the duplication and ensure fixes (e.g., the partial-download issue noted in the other script) are applied in one place.rust/Makefile.am (1)
44-62: Add a comment to.cargo/config.toml.offlinewarning about the fragile append pattern.The offline config relies on
config.toml.offlineending with the[source.vendored-sources]section header. Line 60 ofrust/Makefile.amappendsdirectory = "..."to the copied config file. This works correctly only because.cargo/config.toml.offlineends cleanly with just the section header and a comment. If future editors add content (other sections or key-value pairs) after[source.vendored-sources], the appended line will be placed incorrectly. Add a comment inconfig.toml.offlineto warn against this.configure.ac (1)
1820-1851: LGTM on cross-compilation guards for native tools. All three (NATIVE_AR,NATIVE_CC,NATIVE_CXX) properly enforce that cross-compilation requires explicit values.Minor note:
RUSTUP_TOOLCHAINis substituted (Line 2151) but not included in the diagnostic echo block (Lines 2300–2312), unlike all other Rust variables. Adding it would help debugging.
| AC_DEFUN( | ||
| RS_REQUIRE_PROG, | ||
| [ | ||
| AC_PATH_PROG($1, $2, $2_notfound) | ||
| if test x${$1} = x$2_notfound | ||
| then | ||
| AC_MSG_ERROR("Required program $2 was not found") | ||
| fi | ||
| ] | ||
| ) |
There was a problem hiding this comment.
Minor quoting issues in RS_REQUIRE_PROG macro.
- Line 135: The
testcommand lacks quotes around expansions — paths containing spaces would break the comparison. Should beif test "x${$1}" = "x$2_notfound". - Line 137:
AC_MSG_ERRORargument uses double quotes instead of idiomatic m4 square brackets.
These won't bite for cargo/cxxbridge/rustc in practice, but are worth hardening for macro reuse.
Proposed fix
AC_DEFUN(
RS_REQUIRE_PROG,
[
AC_PATH_PROG($1, $2, $2_notfound)
- if test x${$1} = x$2_notfound
+ if test "x${$1}" = "x$2_notfound"
then
- AC_MSG_ERROR("Required program $2 was not found")
+ AC_MSG_ERROR([Required program $2 was not found])
fi
]
)🤖 Prompt for AI Agents
In `@configure.ac` around lines 131 - 140, The RS_REQUIRE_PROG macro has unsafe
quoting in the test and uses double quotes for AC_MSG_ERROR; update the macro
(around RS_REQUIRE_PROG / AC_PATH_PROG usage) so the comparison is quoted as if
test "x${$1}" = "x${2_notfound}" (i.e., add surrounding double quotes for both
sides) and change the AC_MSG_ERROR argument to use m4 square brackets (e.g.,
AC_MSG_ERROR([Required program $2 was not found])) to avoid m4 quoting issues
and handle paths with spaces safely.
| def download_and_hash(url: str, dest: Path) -> str: | ||
| hasher = hashlib.sha256() | ||
| if dest.exists(): | ||
| print(f" Using existing {dest.name}") | ||
| with open(dest, "rb") as f: | ||
| while chunk := f.read(8192): | ||
| hasher.update(chunk) | ||
| return hasher.hexdigest() | ||
|
|
||
| print(f" Downloading {dest.name}") | ||
| dest.parent.mkdir(parents=True, exist_ok=True) | ||
| with urllib.request.urlopen(url) as response: | ||
| with open(dest, "wb") as f: | ||
| while chunk := response.read(8192): | ||
| hasher.update(chunk) | ||
| f.write(chunk) | ||
| return hasher.hexdigest() |
There was a problem hiding this comment.
Partial download leaves a corrupt file that silently poisons the next run.
If the download is interrupted (network error, timeout, etc.), a partial file remains at dest. On the next invocation, the dest.exists() check at Line 55 will find it, hash the incomplete content, and silently write that wrong hash into the makefile. The depends build will then fail with a confusing checksum mismatch.
Download to a temporary file and rename on success:
Proposed fix
def download_and_hash(url: str, dest: Path) -> str:
hasher = hashlib.sha256()
if dest.exists():
print(f" Using existing {dest.name}")
with open(dest, "rb") as f:
while chunk := f.read(8192):
hasher.update(chunk)
return hasher.hexdigest()
print(f" Downloading {dest.name}")
dest.parent.mkdir(parents=True, exist_ok=True)
- with urllib.request.urlopen(url) as response:
- with open(dest, "wb") as f:
+ tmp = dest.with_suffix(dest.suffix + ".tmp")
+ with urllib.request.urlopen(url, timeout=120) as response:
+ with open(tmp, "wb") as f:
while chunk := response.read(8192):
hasher.update(chunk)
f.write(chunk)
+ tmp.rename(dest)
return hasher.hexdigest()🧰 Tools
🪛 Ruff (0.14.14)
[error] 64-64: Audit URL open for permitted schemes. Allowing use of file: or custom schemes is often unexpected.
(S310)
🤖 Prompt for AI Agents
In `@contrib/devtools/update-rust-hashes.py` around lines 53 - 69, The
download_and_hash function currently treats any existing dest file as complete;
instead perform downloads into a temporary file and atomically move it into
place only on successful completion: open a temp path (e.g.,
dest.with_suffix(".part") or use tempfile) when fetching via
urllib.request.urlopen, write and hash to that temp, then os.replace(temp, dest)
after the loop so partial downloads never overwrite dest; ensure existing dest
is still hashed when present and that the temp file is removed/cleaned up on
exceptions to avoid leaving junk.
| def check_MACHO_sdk(binary) -> bool: | ||
| if binary.build_version.sdk == [14, 0, 0]: | ||
| if binary.build_version.sdk == [15, 0, 0]: | ||
| return True | ||
| return False |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what SDK version is configured in the depends system
# Look for SDK version references in depends makefiles and build scripts
rg -n -i 'sdk.*15' --type-add 'mk:*.mk' --type mk
echo "---"
rg -n 'OSX_SDK_VERSION\|darwin_sdk_version\|SDK_VERSION' depends/ contrib/guix/
echo "---"
rg -n '15\.\(0\|2\)' contrib/guix/ depends/Repository: dashpay/dash
Length of output: 110
Code check is correct, but there's a discrepancy with the PR title. The build system is configured with OSX_SDK_VERSION=15.0 in depends/hosts/darwin.mk, so the check for [15, 0, 0] is correct. However, the PR title states the macOS SDK is being updated to 15.2, which contradicts the actual configuration. Either the PR title is misleading, or the SDK version in depends/hosts/darwin.mk should be updated to 15.2 to match the stated change.
🤖 Prompt for AI Agents
In `@contrib/guix/symbol-check.py` around lines 244 - 247, The PR title, the
check_MACHO_sdk function, and the build config are inconsistent: either update
the SDK in the build system or change the check; specifically, decide whether
OSX_SDK_VERSION should be 15.2 and if so update the build config
(OSX_SDK_VERSION) to 15.2 and change check_MACHO_sdk to compare to [15, 2, 0];
otherwise, change the PR title to say 15.0 (and/or change check_MACHO_sdk to
compare to [15, 0, 0] if you intended 15.0). Ensure consistency between the
build variable OSX_SDK_VERSION, the comparison in check_MACHO_sdk, and the PR
title.
|
Guix Automation has began to build this PR tagged as v23.0.2-devpr7109.409e205a. A new comment will be made when the image is pushed. |
|
Guix Automation has failed with an unknown error for tag v23.0.2-devpr7109.409e205a |
|
same hashes |
3 participants
Additional Information
Depends on backport: merge bitcoin#30423, #27038, #31048, #31099, #31172, #31450, #31608, #31818, #29881, #32458, #32400, #32760, #33178, #33312, #33780, #33181, #34102, partial bitcoin#29023, #30454, #30509, #30510, #31105, #32922, #33489, #33445 (build backports: part 5) #6927
To ensure our list of targets validated are matched with targets used for releases (see
guix-start), we switch outarm-linux-gnueabihfwithaarch64-linux-gnu, this is now reflected by changes to our GitHub CI configuration, build scripts, Guix CI action.On top of
ntdll.dll, Rust's standard library has a hard dependency onapi-ms-win-core-synch-l1-2-0.dll(sync primitives) after rust-lang/rust#124019, which has been added to the allowlist even though this dependency isn't specified inRUST_LIBS.The build system passes information about the host compiler, target macOS version and other parameters as Rust crates themselves may be FFI bindings to C/C++ codebases, this is specifically relevant for cross-compilation where host and target do not match.
dependsexposes whatever is needed but if attempting to build withoutdepends, the following environment variables need to be defined.OSX_MIN_VERSION(target version of macOS)OSX_SDK(path to the macOS SDK)NATIVE_AR(host archiver)NATIVE_CC(host C compiler)NATIVE_CXX(host C++ compiler)RUST_NATIVE(host Rust target, if cannot be deduced by build system)For integrating the crates into our C++ codebase, we use the
cxxcrate with the version defined innative_cxxbridge.mkAs mentioned in dash#6927, we need to update the macOS SDK to ensure we have standards-conformant definitions to deal with dtolnay/cxx#1574.
This does result in us using a higher SDK version than used upstream, requiring us to update the source URLs to our own fallback servers as upstream is not expected to host this version of the consolidated SDK. This has no effect on the target version of macOS, which will continue to be macOS 14.
The version of Rust defined in
native_rust.mkandrust-toolchain.tomlmust match as the former is what we use independs(and is the same version used in the Guix environment, we don't rely on Guix to supply us a Rust build environment) and the latter is used to ensure that builds outsidedependsuse a matching Rust version.The version should also be synced with
rust_stdlib.mk, which defines the standard libraries used. All hosts (native_rust) will have a target (rust_stdlib) but not all targets will have a host. Version management is done usingcontrib/devtools/update-rust-hashes.pyandcontrib/devtools/update-native-cxxbridge.py.To avoid glibc-related issues in Guix builds due to conflicting versions, incompatible symbols or the mixing of code generated with different glibc targets, we have opted to use the
muslvariant of the standard library for Linux builds. Non-dependsbuilds will need to configure their Rust environment accordingly.To allow PowerPC target builds, we had to move from Rust 1.82 (the minimum version demanded by
cxx) to 1.85.1This also means that the CI Docker image does not have a Rust compiler as it is supplied by
depends. This is done due to the fact that we already need a depends-supplied compiler to have greater control over version and variant but also to avoid increasing the size of our already-large master image.Future work on the CI system could provide Rust compilers to aid in debugging and development (as development images are derived from CI images) but that is out of scope for this PR.
The depends system introduces the following verbs
download-rust-src(downloads packages necessary for vendoring)download-rust-std(downloads the standard library for all targets listed inrust_stdlib.mk, needed as Guix environments do not have access to the internet)vendor-dep-crates(vendors dependencies by Rust packages in thedependssystem)vendor-all-crates(vendors dependencies by Rust packages used in the codebase)During development and when building without the depends system, you will need to use
--enable-online-rust, failing which, you will need to defineRUST_VENDORED_SOURCES(a value that is otherwise defined by thedependssystem).Vendoring support was added as Guix environments have no access to the internet and to allow for caching crates for CI.
As the Guix environment does not use normal paths, we have to do the inverse of the patch that is usually done to binaries generated in Guix environments (replacing the loader with standard paths instead of store-specific paths) to ensure our native binaries will run and additionally locate the GCC runtime (
libgcc_s.so.1) and move it to a location reachable to the native binaries and patch that patch too.This is done using
contrib/guix/libexec/fix-elf-interpreter.shand is meant only for the Guix environment as it assumes that it will always run on a Linux host.How Has This Been Tested?
Running
dashdordash-qtshould have this entry in their debug log (see below) demonstrating a call to the stub crate during init.Breaking Changes
None expected.
Checklist