Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the Pants native client in released wheels #18957

Merged
merged 3 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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}" "$@"
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you intend to get some mileage on _PANTS_SERVER_EXE here? Seems good to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't (yet): it's mentioned in the description as something that would be worth doing. But I think that doing it requires significant refactoring of this script to split it into pants vs pants_server scripts.

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