Skip to content

Commit

Permalink
Include the Pants native client in released wheels (pantsbuild#18957)
Browse files Browse the repository at this point in the history
The Pants native client which was introduced in pantsbuild#11922 has so far only
been usable in the Pants repo when the `USE_NATIVE_PANTS` environment
variable was set.

To make the native client available to end users, this change begins
distributing the native-client binary in Pants wheels. A corresponding
change in the `scie-pants` repo
(pantsbuild/scie-pants#172) uses the native
client to run `pants`.

To reduce the surface area of `scie-pants` (rather than having it be
responsible for handling the special `75` exit code similar to the
`pants` script integration), this PR also moves to having the
native-client execute its own fallback (via `execv`) to the Pants
entrypoint. In future, the `pantsbuild/pants` `pants` script could also
use that facility (e.g. by specifying a separate `pants_server` bash
script as the entrypoint to use as the `_PANTS_SERVER_EXE`).

----

As originally demonstrated on pantsbuild#11831, the native client is still much
faster than the legacy client. Using
pantsbuild/scie-pants#172, the timings look
like:
```
Benchmark #1: PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):      1.161 s ±  0.067 s    [User: 830.6 ms, System: 79.2 ms]
  Range (min … max):    1.054 s …  1.309 s    10 runs

Benchmark #2: PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help
  Time (mean ± σ):     271.0 ms ±  30.6 ms    [User: 8.9 ms, System: 6.9 ms]
  Range (min … max):   241.5 ms … 360.6 ms    12 runs

Summary
  'PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help' ran
    4.29 ± 0.54 times faster than 'PANTS_NO_NATIVE_CLIENT=true PANTS_SHA=836cceb74e6af042e7c82677f3ceb4927efce20e scie-pants-macos-x86_64 help'
```

Fixes pantsbuild#11831.
  • Loading branch information
stuhood authored May 10, 2023
1 parent df749c1 commit 558d843
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 36 deletions.
12 changes: 6 additions & 6 deletions .github/workflows/test-cron.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
uses: actions/cache@v3
with:
key: Linux-ARM64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -90,7 +90,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.Linux-ARM64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -155,7 +155,7 @@ jobs:
uses: actions/cache@v3
with:
key: Linux-x86_64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -191,7 +191,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -268,7 +268,7 @@ jobs:
uses: actions/cache@v3
with:
key: macOS11-x86_64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -304,7 +304,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.macOS11-x86_64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down
12 changes: 6 additions & 6 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ jobs:
uses: actions/cache@v3
with:
key: Linux-ARM64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -93,7 +93,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.Linux-ARM64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -158,7 +158,7 @@ jobs:
uses: actions/cache@v3
with:
key: Linux-x86_64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -193,7 +193,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -270,7 +270,7 @@ jobs:
uses: actions/cache@v3
with:
key: macOS11-x86_64-engine-${{ steps.get-engine-hash.outputs.hash }}-v1
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down Expand Up @@ -305,7 +305,7 @@ jobs:
uses: actions/upload-artifact@v3
with:
name: native_binaries.${{ matrix.python-version }}.macOS11-x86_64
path: '.pants
path: 'src/python/pants/bin/native_client
src/python/pants/engine/internals/native_engine.so
Expand Down
2 changes: 1 addition & 1 deletion build-support/bin/generate_github_workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def hashFiles(path: str) -> str:


NATIVE_FILES = [
".pants",
"src/python/pants/bin/native_client",
"src/python/pants/engine/internals/native_engine.so",
"src/python/pants/engine/internals/native_engine.so.metadata",
]
Expand Down
8 changes: 4 additions & 4 deletions build-support/bin/rust/bootstrap_code.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ case "${KERNEL}" in
esac

readonly NATIVE_ENGINE_BINARY="native_engine.so"
export NATIVE_CLIENT_BINARY="${REPO_ROOT}/src/python/pants/bin/native_client"
readonly NATIVE_ENGINE_RESOURCE="${REPO_ROOT}/src/python/pants/engine/internals/${NATIVE_ENGINE_BINARY}"
readonly NATIVE_ENGINE_RESOURCE_METADATA="${NATIVE_ENGINE_RESOURCE}.metadata"
readonly NATIVE_CLIENT_PATH="${REPO_ROOT}/.pants"
readonly NATIVE_CLIENT_TARGET="${NATIVE_ROOT}/target/${MODE}/pants"

function _build_native_code() {
Expand All @@ -55,7 +55,7 @@ function bootstrap_native_code() {
engine_version_in_metadata="$(sed -n 's/^engine_version: //p' "${NATIVE_ENGINE_RESOURCE_METADATA}")"
fi

if [[ -f "${NATIVE_ENGINE_RESOURCE}" && -f "${NATIVE_CLIENT_PATH}" &&
if [[ -f "${NATIVE_ENGINE_RESOURCE}" && -f "${NATIVE_CLIENT_BINARY}" &&
"${engine_version_calculated}" == "${engine_version_in_metadata}" ]]; then
return 0
fi
Expand All @@ -79,9 +79,9 @@ function bootstrap_native_code() {
# Create the native engine resource.
# NB: On Mac Silicon, for some reason, first removing the old native_engine.so is necessary to avoid the Pants
# process from being killed when recompiling.
rm -f "${NATIVE_ENGINE_RESOURCE}" "${NATIVE_CLIENT_PATH}"
rm -f "${NATIVE_ENGINE_RESOURCE}" "${NATIVE_CLIENT_BINARY}"
cp "${native_binary}" "${NATIVE_ENGINE_RESOURCE}"
cp "${NATIVE_CLIENT_TARGET}" "${NATIVE_CLIENT_PATH}"
cp "${NATIVE_CLIENT_TARGET}" "${NATIVE_CLIENT_BINARY}"

# Create the accompanying metadata file.
local -r metadata_file=$(mktemp -t pants.native_engine.metadata.XXXXXX)
Expand Down
23 changes: 11 additions & 12 deletions pants
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,25 @@ source "${HERE}/build-support/pants_venv"
source "${HERE}/build-support/bin/rust/bootstrap_code.sh"

function exec_pants_bare() {
PANTS_NATIVE_EXE="${HERE}/.pants"
PANTS_PY_EXE="${HERE}/src/python/pants/bin/pants_loader.py"
PANTS_SRCPATH="${HERE}/src/python"

# Redirect activation and native bootstrap to ensure that they don't interfere with stdout.
activate_pants_venv 1>&2
bootstrap_native_code 1>&2

if [ -n "${USE_NATIVE_PANTS}" ]; then
if [ -n "${PANTS_DEBUG}" ]; then
if [[ "$*" != *"--no-pantsd"* ]]; then
echo "Error! Must pass '--no-pantsd' when using PANTS_DEBUG"
exit 1
fi
DEBUG_ARGS="-m debugpy --listen 127.0.0.1:5678 --wait-for-client"
echo "Will launch debugpy server at '127.0.0.1:5678' waiting for client connection."
fi

if [ -z "${PANTS_NO_NATIVE_CLIENT}" ]; then
set +e
"${PANTS_NATIVE_EXE}" "$@"
"${NATIVE_CLIENT_BINARY}" "$@"
result=$?
# N.B.: The native pants client currently relies on pantsd being up. If it's not, it will fail
# with exit code 75 (EX_TEMPFAIL in /usr/include/sysexits.h) and we should fall through to the
Expand All @@ -63,15 +71,6 @@ function exec_pants_bare() {
set -e
fi

if [ -n "${PANTS_DEBUG}" ]; then
if [[ "$*" != *"--no-pantsd"* ]]; then
echo "Error! Must pass '--no-pantsd' when using PANTS_DEBUG"
exit 1
fi
DEBUG_ARGS="-m debugpy --listen 127.0.0.1:5678 --wait-for-client"
echo "Will launch debugpy server at '127.0.0.1:5678' waiting for client connection."
fi

# shellcheck disable=SC2086
PYTHONPATH="${PANTS_SRCPATH}:${PYTHONPATH}" RUNNING_PANTS_FROM_SOURCES=1 NO_SCIE_WARNING=1 \
exec ${PANTS_PREPEND_ARGS:-} "$(venv_dir)/bin/python" ${DEBUG_ARGS} "${PANTS_PY_EXE}" "$@"
Expand Down
2 changes: 2 additions & 0 deletions src/python/pants/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ python_distribution(
dependencies=[
"./__main__.py",
":resources",
# Include the native client binary in the distribution.
"src/python/pants/bin:native_client",
],
# Because we have native code, this will cause the wheel to use whatever the ABI is for the
# interpreter used to run setup.py, e.g. `cp36m-macosx_10_15_x86_64`.
Expand Down
1 change: 1 addition & 0 deletions src/python/pants/bin/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/native_client
4 changes: 4 additions & 0 deletions src/python/pants/bin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -99,5 +99,9 @@ pex_binary(
strip_pex_env=False,
)

resources(
name="native_client",
sources=["native_client"],
)

python_tests(name="tests")
45 changes: 41 additions & 4 deletions src/rust/engine/client/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,16 @@
// Arc<Mutex> can be more clear than needing to grok Orderings:
#![allow(clippy::mutex_atomic)]

use std::convert::AsRef;
use std::convert::{AsRef, Infallible};
use std::env;
use std::ffi::{CString, OsString};
use std::os::unix::ffi::OsStringExt;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::time::SystemTime;

use log::debug;
use nix::unistd::execv;
use strum::VariantNames;
use strum_macros::{AsRefStr, EnumString, EnumVariantNames};

Expand Down Expand Up @@ -135,23 +138,57 @@ fn find_pantsd(
Ok(pantsd_settings)
}

fn execv_fallback_client(pants_server: OsString) -> Result<Infallible, i32> {
let exe = PathBuf::from(pants_server.clone());
let c_exe = CString::new(exe.into_os_string().into_vec())
.expect("Failed to convert executable to a C string.");

let mut c_args = vec![c_exe.clone()];
c_args.extend(
std::env::args_os()
.skip(1)
.map(|arg| CString::new(arg.into_vec()).expect("Failed to convert argument to a C string.")),
);

execv(&c_exe, &c_args).map_err(|errno| {
eprintln!("Failed to exec pants at {pants_server:?}: {}", errno.desc());
1
})
}

// The value is taken from this C precedent:
// ```
// $ grep 75 /usr/include/sysexits.h
// #define EX_TEMPFAIL 75 /* temp failure; user is invited to retry */
// ```
const EX_TEMPFAIL: i32 = 75;

// An environment variable which if set, points to a non-native entrypoint to fall back to if
// `pantsd` is not already running with the appropriate fingerprint.
//
// This environment variable constitutes a public API used by `scie-pants` and the `pants` script.
// But in future, the native client may become the only client for `pantsd` (by directly handling
// forking the `pantsd` process and then connecting to it).
const PANTS_SERVER_EXE: &str = "_PANTS_SERVER_EXE";

#[tokio::main]
async fn main() {
let start = SystemTime::now();
match execute(start).await {
Err(err) => {
let pants_server = env::var_os(PANTS_SERVER_EXE);
match (execute(start).await, pants_server) {
(Err(_), Some(pants_server)) => {
// We failed to connect to `pantsd`, but a server variable was provided. Fall back
// to `execv`'ing the legacy Python client, which will handle spawning `pantsd`.
if let Err(exit_code) = execv_fallback_client(pants_server) {
std::process::exit(exit_code);
}
}
(Err(err), None) => {
eprintln!("{err}");
// We use this exit code to indicate an error running pants via the nailgun protocol to
// differentiate from a successful nailgun protocol session.
std::process::exit(EX_TEMPFAIL);
}
Ok(exit_code) => std::process::exit(exit_code),
(Ok(exit_code), _) => std::process::exit(exit_code),
}
}
2 changes: 1 addition & 1 deletion src/rust/engine/options/src/build_root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use log::debug;
pub struct BuildRoot(PathBuf);

impl BuildRoot {
const SENTINEL_FILES: &'static [&'static str] = &["pants", "BUILDROOT", "BUILD_ROOT"];
const SENTINEL_FILES: &'static [&'static str] = &["pants.toml", "BUILDROOT", "BUILD_ROOT"];

pub fn find() -> Result<BuildRoot, String> {
let cwd = env::current_dir().map_err(|e| format!("Failed to determine $CWD: {e}"))?;
Expand Down
4 changes: 2 additions & 2 deletions src/rust/engine/options/src/build_root_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn test_find_cwd() {

assert_sentinel("BUILDROOT");
assert_sentinel("BUILD_ROOT");
assert_sentinel("pants");
assert_sentinel("pants.toml");
}

#[test]
Expand All @@ -44,7 +44,7 @@ fn test_find_subdir() {
assert!(BuildRoot::find_from(&buildroot_path).is_err());
assert!(BuildRoot::find_from(&subdir).is_err());

let sentinel = &buildroot.path().join("pants");
let sentinel = &buildroot.path().join("pants.toml");
fs::write(sentinel, []).unwrap();
assert_eq!(
&buildroot_path,
Expand Down

0 comments on commit 558d843

Please sign in to comment.