Copilot commented on code in PR #55491: URL: https://github.com/apache/arrow/pull/55491#discussion_r4538285635
########## r/tools/trim/boost.sh: ########## @@ -0,0 +1,205 @@ +#!/usr/bin/env bash +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Trim the Boost CMake source tarball (boost-X.Y.Z-cmake.tar.gz, ~140 MB) +# down to the Boost libraries that libarrow's bundled Boost build +# (build_boost() in cpp/cmake_modules/ThirdpartyToolchain.cmake) uses when +# the R package builds libarrow, so it is small enough to ship in the R +# package (~5 MB). +# +# Usage: tools/trim/boost.sh IN_TARBALL OUT_TARBALL +# +# Called by tools/vendor_dependencies.sh (`make sync-cpp`) after tools/cpp +# has been synced. Needs cmake and a C++ compiler, because Boost's own CMake +# is used to work out which libraries are needed. +# +# How it works: +# +# 1. Read the libraries build_boost() adds to BOOST_INCLUDE_LIBRARIES from +# ThirdpartyToolchain.cmake, minus the ones that the R build never needs +# (EXCLUDED_LIBRARIES below). +# 2. Configure Boost with that BOOST_INCLUDE_LIBRARIES. Boost's CMake adds +# the transitive dependencies and logs each library it adds. +# 3. Keep only the top-level files, tools/cmake and those libraries, without +# their tests, docs, examples, etc. +# +# Set BOOST_INCLUDE_LIBRARIES (semicolon-separated) in the environment to +# skip step 1 and use that list instead. + +set -euo pipefail + +if [ "$#" -ne 2 ]; then + echo "Usage: $0 IN_TARBALL OUT_TARBALL" >&2 + exit 1 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +IN="$(cd "$(dirname "$1")" && pwd)/$(basename "$1")" +OUT="$(cd "$(dirname "$2")" && pwd)/$(basename "$2")" + +# Libraries that build_boost() can add to BOOST_INCLUDE_LIBRARIES but that +# the R package's libarrow build never needs. If you add a library here, +# say why. +# +# - multiprecision: only needed when ARROW_BOOST_NEED_MULTIPRECISION is +# true, i.e. with ARROW_TESTING, ARROW_GANDIVA, ARROW_USE_UBSAN or a +# compiler without a native 128-bit integer (ARROW_USE_NATIVE_INT128). +# The R package builds none of those, and gcc/clang on the 64-bit +# platforms that nixlibs.R builds for all have __int128 (32-bit targets +# don't). Including it would more than double the size of the trimmed +# tarball (~5 MB, 54 libraries -> ~11 MB, 58 libraries). If a build that +# needs it uses this tarball, configuring libarrow fails in build_boost() +# because the boost_multiprecision target doesn't exist. +# - beast, variant, xpressive: only for ARROW_FLIGHT_SQL_ODBC. +EXCLUDED_LIBRARIES=(multiprecision beast variant xpressive) Review Comment: Building from `tools/cpp` with `EXTRA_CMAKE_FLAGS=-DARROW_USE_UBSAN=ON` fails when bundled Boost is used: CMake disables native `int128` and requires the excluded `boost_multiprecision` target. The vendored archive is selected even when downloads are available. Retain multiprecision, or bypass the trimmed Boost archive for configurations that need it and account for its absence in offline builds. ########## r/tools/nixlibs.R: ########## @@ -902,9 +970,68 @@ with_vendored_dependencies <- function(env_var_list) { } next } - env_var_list <- replace(env_var_list, env_varname, normalizePath(tarball)) + checksum_varname <- sub("_URL$", "_BUILD_SHA256_CHECKSUM", env_varname) + entry <- manifest[manifest$filename == filename, , drop = FALSE] + # versions.txt has the vendored checksum if use_vendored_checksums() has + # already run on it (e.g. when installing from the r/ directory again) + matches <- nrow(entry) == 1 && + unname(expected[checksum_varname]) %in% c(entry$upstream_sha256, entry$vendored_sha256) + if (!matches) { + if (!quietly) { + lg( + "Vendored source for %s doesn't match %s, it will be downloaded if needed", + env_varname, + file.path(src_dir, "thirdparty", "versions.txt"), + .indent = "****" + ) + } + next + } + usable[[env_varname]] <- normalizePath(tarball) } - env_var_list + usable +} + +# Read the checksums.txt that tools/vendor_dependencies.sh writes, with the +# checksums of each vendored tarball before (upstream) and after trimming +read_vendored_checksums <- function() { + manifest_file <- file.path(vendored_dependency_dir, "checksums.txt") + if (!file.exists(manifest_file)) { + return(NULL) + } + read.table(manifest_file, header = TRUE, stringsAsFactors = FALSE) +} + +# The libarrow build checks the checksums of the thirdparty tarballs against +# src_dir's versions.txt, so replace the upstream checksums of the vendored +# tarballs that `env_var_list` points at with the trimmed tarballs' checksums. +use_vendored_checksums <- function(src_dir, env_var_list, vendored) { + used <- vendored[vapply( + names(vendored), + function(env_varname) identical(env_var_list[[env_varname]], vendored[[env_varname]]), + logical(1) + )] + if (length(used) == 0) { + return(invisible()) + } + manifest <- read_vendored_checksums() + versions_txt <- file.path(src_dir, "thirdparty", "versions.txt") + lines <- readLines(versions_txt) + for (env_varname in names(used)) { + checksum_varname <- sub("_URL$", "_BUILD_SHA256_CHECKSUM", env_varname) + checksum <- manifest$vendored_sha256[manifest$filename == basename(used[[env_varname]])] + lines <- sub(paste0("^", checksum_varname, "=.*$"), paste0(checksum_varname, "=", checksum), lines) + } + writeLines(lines, versions_txt) Review Comment: The checksum rewrite persists across builds, but never restores upstream checksums for dependencies that stop using the vendored tarball. After one build from tools/cpp, rebuilding with an upstream ARROW_*_URL override or with set_thirdparty_urls() leaves the trimmed checksum in versions.txt, so CMake rejects the original archive with a hash mismatch. Restore each matching manifest entry's upstream checksum when its vendored archive is not selected, or scope these edits to the current build and restore the file afterward. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
