Move disabling the native-client into the native client itself (pants…

…build#19010) On pantsbuild/scie-pants#172, @jsirois pointed out that allowing the native-client to handle disabling itself (if requested) would be much cleaner from a `scie-pants` perspective, and keeping `scie-pants` as small and simple as possible is an important goal. This change moves `PANTS_NO_NATIVE_CLIENT` handling into the native client.
thejcannon · May 15, 2023 · 731abd8 · 731abd8
1 parent 5c74e52
commit 731abd8
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 16 deletions.
diff --git a/.github/workflows/test-cron.yaml b/.github/workflows/test-cron.yaml
@@ -362,6 +362,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -415,6 +418,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-ARM64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -487,6 +493,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -559,6 +568,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -631,6 +643,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -688,6 +703,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.macOS11-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV

diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml
@@ -655,6 +655,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -746,6 +749,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-ARM64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -817,6 +823,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -888,6 +897,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -959,6 +971,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.Linux-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV
@@ -1016,6 +1031,9 @@ jobs:
       uses: actions/download-artifact@v3
       with:
         name: native_binaries.${{ matrix.python-version }}.macOS11-x86_64
+        path: src/python/pants
+    - name: Make native-client runnable
+      run: chmod +x src/python/pants/bin/native_client
     - if: github.event_name != 'pull_request'
       name: Setup toolchain auth
       run: 'echo TOOLCHAIN_AUTH_TOKEN="${{ secrets.TOOLCHAIN_AUTH_TOKEN }}" >> $GITHUB_ENV

diff --git a/build-support/bin/generate_github_workflows.py b/build-support/bin/generate_github_workflows.py
@@ -67,10 +67,14 @@ def hashFiles(path: str) -> str:
 # ----------------------------------------------------------------------
 
 
+# NB: The `upload-artifact` action strips the longest common prefix of paths in the
+# created artifact, but the `download-artifact` action needs to know what that prefix
+# was.
+NATIVE_FILES_COMMON_PREFIX = "src/python/pants"
 NATIVE_FILES = [
-    "src/python/pants/bin/native_client",
-    "src/python/pants/engine/internals/native_engine.so",
-    "src/python/pants/engine/internals/native_engine.so.metadata",
+    f"{NATIVE_FILES_COMMON_PREFIX}/bin/native_client",
+    f"{NATIVE_FILES_COMMON_PREFIX}/engine/internals/native_engine.so",
+    f"{NATIVE_FILES_COMMON_PREFIX}/engine/internals/native_engine.so.metadata",
 ]
 
 # We don't specify patch versions so that we get the latest, which comes pre-installed:
@@ -344,14 +348,21 @@ def native_binaries_upload(self) -> Step:
             },
         }
 
-    def native_binaries_download(self) -> Step:
-        return {
-            "name": "Download native binaries",
-            "uses": "actions/download-artifact@v3",
-            "with": {
-                "name": f"native_binaries.{gha_expr('matrix.python-version')}.{self.platform_name()}",
+    def native_binaries_download(self) -> Sequence[Step]:
+        return [
+            {
+                "name": "Download native binaries",
+                "uses": "actions/download-artifact@v3",
+                "with": {
+                    "name": f"native_binaries.{gha_expr('matrix.python-version')}.{self.platform_name()}",
+                    "path": NATIVE_FILES_COMMON_PREFIX,
+                },
             },
-        }
+            {
+                "name": "Make native-client runnable",
+                "run": f"chmod +x {NATIVE_FILES[0]}",
+            },
+        ]
 
     def rust_caches(self) -> Sequence[Step]:
         return [
@@ -647,7 +658,7 @@ def test_jobs(
             ),
             *helper.setup_primary_python(),
             *helper.expose_all_pythons(),
-            helper.native_binaries_download(),
+            *helper.native_binaries_download(),
             setup_toolchain_auth(),
             {
                 "name": human_readable_step_name,
@@ -810,7 +821,7 @@ def test_workflow_jobs(python_versions: list[str], *, cron: bool) -> Jobs:
                 "steps": [
                     *checkout(),
                     *linux_x86_64_helper.setup_primary_python(),
-                    linux_x86_64_helper.native_binaries_download(),
+                    *linux_x86_64_helper.native_binaries_download(),
                     setup_toolchain_auth(),
                     {
                         "name": "Lint",

diff --git a/pants b/pants
@@ -65,6 +65,10 @@ function exec_pants_bare() {
     # with exit code 75 (EX_TEMPFAIL in /usr/include/sysexits.h) and we should fall through to the
     # python pants client which knows how to start up pantsd. This failure takes O(1ms); so has no
     # appreciable impact on --no-pantsd runs.
+    #
+    # TODO: Split out a `pants_server` or `pants_legacy_entrypoint` from this script, and then use
+    # the native client's support for fallback to the legacy entrypoint to remove the special
+    # exit code case.
     if ((result != 75)); then
       exit ${result}
     fi

diff --git a/src/rust/engine/client/src/main.rs b/src/rust/engine/client/src/main.rs
@@ -138,7 +138,7 @@ fn find_pantsd(
   Ok(pantsd_settings)
 }
 
-fn execv_fallback_client(pants_server: OsString) -> Result<Infallible, i32> {
+fn try_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.");
@@ -156,6 +156,13 @@ fn execv_fallback_client(pants_server: OsString) -> Result<Infallible, i32> {
   })
 }
 
+fn execv_fallback_client(pants_server: OsString) -> Infallible {
+  if let Err(exit_code) = try_execv_fallback_client(pants_server) {
+    std::process::exit(exit_code);
+  }
+  unreachable!()
+}
+
 // The value is taken from this C precedent:
 // ```
 // $ grep 75 /usr/include/sysexits.h
@@ -170,18 +177,32 @@ const EX_TEMPFAIL: i32 = 75;
 // 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";
+// An end-user-settable environment variable to skip attempting to use the native client, and
+// immediately delegate to the legacy client.
+const PANTS_NO_NATIVE_CLIENT: &str = "PANTS_NO_NATIVE_CLIENT";
 
 #[tokio::main]
 async fn main() {
   let start = SystemTime::now();
+  let no_native_client =
+    matches!(env::var_os(PANTS_NO_NATIVE_CLIENT), Some(value) if !value.is_empty());
   let pants_server = env::var_os(PANTS_SERVER_EXE);
+
+  match &pants_server {
+    Some(pants_server) if no_native_client => {
+      // The user requested that the native client not be used. Immediately fall back to the legacy
+      // client.
+      execv_fallback_client(pants_server.clone());
+      return;
+    }
+    _ => {}
+  }
+
   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);
-      }
+      execv_fallback_client(pants_server);
     }
     (Err(err), None) => {
       eprintln!("{err}");