Skip to content

fix(deps-dev): Ensure user-scoped pipx installs and reliable version checks for pipx packages. #1320

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.

Sign up for GitHub

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

fix(deps-dev): Ensure user-scoped pipx installs and reliable version checks for pipx packages. #1320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e6817d8
remove sudo when installing pipx packages
Bill-hbrhbr Sep 23, 2025
1701f05
Apply suggestions from code review
Bill-hbrhbr Sep 23, 2025
896a9c4
line break for lint
Bill-hbrhbr Sep 23, 2025
12fc072
Preserve all env vars when running pipx install as user
Bill-hbrhbr Sep 23, 2025
24adfec
Add pipx-packages to trigger various build CI pipelines
Bill-hbrhbr Sep 23, 2025
84c01bf
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Sep 23, 2025
22ec79e
fix issue where the installed binary is not immediately available for…
Bill-hbrhbr Sep 25, 2025
59de3b0
Find installed pipx binary in the more reliable way
Bill-hbrhbr Sep 25, 2025
611596a
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Sep 25, 2025
ac5ec93
fix bug
Bill-hbrhbr Sep 25, 2025
d547a7c
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Sep 27, 2025
6bea8cf
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Sep 28, 2025
7523533
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Oct 1, 2025
4640164
Merge branch 'main' into sudo-install-lib
Bill-hbrhbr Oct 2, 2025
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
4 changes: 4 additions & 0 deletions .github/workflows/clp-artifact-build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,24 +62,28 @@ jobs:
- ".github/actions/**"
- ".github/workflows/clp-artifact-build.yaml"
- "components/core/tools/scripts/lib_install/*.sh"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/docker-images/clp-env-base-centos-stream-9/**"
- "components/core/tools/scripts/lib_install/centos-stream-9/**"
manylinux_2_28_x86_64_image:
- ".github/actions/**"
- ".github/workflows/clp-artifact-build.yaml"
- "components/core/tools/scripts/lib_install/*.sh"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/**"
- "components/core/tools/scripts/lib_install/manylinux_2_28/**"
musllinux_1_2_x86_64_image:
- ".github/actions/**"
- ".github/workflows/clp-artifact-build.yaml"
- "components/core/tools/scripts/lib_install/*.sh"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/**"
- "components/core/tools/scripts/lib_install/musllinux_1_2/**"
ubuntu_jammy_image:
- ".github/actions/**"
- ".github/workflows/clp-artifact-build.yaml"
- "components/core/tools/scripts/lib_install/*.sh"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/docker-images/clp-env-base-ubuntu-jammy/**"
- "components/core/tools/scripts/lib_install/ubuntu-jammy/**"
clp:
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/clp-core-build-macos.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ on:
- "components/core/CMakeLists.txt"
- "components/core/src/**"
- "components/core/tests/**"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/scripts/lib_install/macos/**"
- "components/core/tools/scripts/deps-download/**"
- "components/core/tools/scripts/utils/build-and-run-unit-tests.py"
Expand All @@ -23,6 +24,7 @@ on:
- "components/core/CMakeLists.txt"
- "components/core/src/**"
- "components/core/tests/**"
- "components/core/tools/scripts/lib_install/pipx-packages/**"
- "components/core/tools/scripts/lib_install/macos/**"
- "components/core/tools/scripts/deps-download/**"
- "components/core/tools/scripts/utils/build-and-run-unit-tests.py"
Expand Down
28 changes: 28 additions & 0 deletions components/core/tools/scripts/lib_install/pipx-packages/find-pipx-bin.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
#!/usr/bin/env bash

set -o errexit
set -o nounset
set -o pipefail

if ! command -v pipx >/dev/null 2>&1; then
echo "Error: pipx not found."
exit 1
fi

if ! command -v jq >/dev/null 2>&1; then
echo "Error: jq not found."
exit 1
fi

if [ "$#" -ne 2 ] || [ -z "${1:-}" ] || [ -z "${2:-}" ]; then
echo "Usage: $0 <package_name> <binary_name>" >&2
exit 2
fi

pkg="$1"
app="$2"

pipx list --json | jq --raw-output --arg pkg "$pkg" --arg app "$app" '
.venvs[$pkg].metadata.main_package.app_paths[]?.__Path__
| select((split("/") | last) == $app)
' | head -n1
7 changes: 7 additions & 0 deletions components/core/tools/scripts/lib_install/pipx-packages/install-all.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ set -o errexit
set -o nounset
set -o pipefail

is_sudo_from_non_root=$(( EUID == 0 && ${SUDO_UID:-0} != 0 ))
if (( is_sudo_from_non_root )); then
echo "Installing pipx packages to the user environment (sudo lifted)."
exec sudo --preserve-env --set-home --user="$SUDO_USER" \
/usr/bin/env bash "${BASH_SOURCE[0]}" "$@"
fi

script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"

"${script_dir}/install-cmake.sh"
Expand Down
18 changes: 13 additions & 5 deletions components/core/tools/scripts/lib_install/pipx-packages/install-cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,24 @@ readonly required_version_min="${required_version_major_min}.${required_version_
readonly required_version_major_max=3
readonly required_version_major_max_plus_1=$((required_version_major_max + 1))

package_preinstalled=0
if ! command -v cmake >/dev/null 2>&1; then
cmake_bin="$(command -v cmake 2>/dev/null || true)"
if [ -n "${cmake_bin}" ]; then
package_preinstalled=0
echo "Preinstalled CMake found at: ${cmake_bin}"
else
package_preinstalled=1
# ystdlib requires CMake v3.23; ANTLR and yaml-cpp do not yet support CMake v4+
# (see https://github.com/y-scope/clp/issues/795).
pipx install --force "cmake>=${required_version_min},<${required_version_major_max_plus_1}"
pipx ensurepath
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
cmake_bin=$("${script_dir}/find-pipx-bin.sh" cmake cmake)
echo "Pipx CMake installed at: ${cmake_bin}"
fi
Comment on lines +28 to 31
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate resolved cmake_bin after pipx install.

If the helper fails to locate the binary, subsequent invocations will crash obscurely.

-    script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
-    cmake_bin=$("${script_dir}/find-pipx-bin.sh" cmake cmake)
-    echo "Pipx CMake installed at: ${cmake_bin}"
+    script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
+    cmake_bin=$("${script_dir}/find-pipx-bin.sh" cmake cmake)
+    if [ -z "${cmake_bin}" ] || [ ! -x "${cmake_bin}" ]; then
+        echo "Error: could not resolve CMake binary after pipx install." >&2
+        echo "       Try: pipx uninstall cmake && pipx install 'cmake>=${required_version_min},<${required_version_major_max_plus_1}'" >&2
+        exit 1
+    fi
+    echo "Pipx CMake installed at: ${cmake_bin}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
cmake_bin=$("${script_dir}/find-pipx-bin.sh" cmake cmake)
echo "Pipx CMake installed at: ${cmake_bin}"
fi
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
cmake_bin=$("${script_dir}/find-pipx-bin.sh" cmake cmake)
if [ -z "${cmake_bin}" ] || [ ! -x "${cmake_bin}" ]; then
echo "Error: could not resolve CMake binary after pipx install." >&2
echo " Try: pipx uninstall cmake && pipx install 'cmake>=${required_version_min},<${required_version_major_max_plus_1}'" >&2
exit 1
fi
echo "Pipx CMake installed at: ${cmake_bin}"
fi
🤖 Prompt for AI Agents 
In components/core/tools/scripts/lib_install/pipx-packages/install-cmake.sh
around lines 28 to 31, the script does not validate the resolved cmake_bin
returned by find-pipx-bin.sh; if it is empty or not executable subsequent calls
will fail obscurely — after calling find-pipx-bin.sh check that cmake_bin is
non-empty and points to an executable file, and if not print a clear error
message to stderr and exit with a non-zero status so the failure is explicit
(optionally include guidance to re-run pipx install or check PATH).


installed_version=$(cmake -E capabilities | jq --raw-output ".version.string")
installed_version_major=$(cmake -E capabilities | jq --raw-output ".version.major")
installed_version_minor=$(cmake -E capabilities | jq --raw-output ".version.minor")
installed_version=$("${cmake_bin}" -E capabilities | jq --raw-output ".version.string")
installed_version_major=$("${cmake_bin}" -E capabilities | jq --raw-output ".version.major")
installed_version_minor=$("${cmake_bin}" -E capabilities | jq --raw-output ".version.minor")

# ystdlib requires CMake v3.23; ANTLR and yaml-cpp do not yet support CMake v4+
# (see https://github.com/y-scope/clp/issues/795).
Expand All @@ -47,3 +53,5 @@ if (("${installed_version_major}" < "${required_version_major_min}")) \

exit 1
fi

echo "CMake version ${installed_version} installed at ${cmake_bin} satisfies version requirements."
20 changes: 14 additions & 6 deletions components/core/tools/scripts/lib_install/pipx-packages/install-go-task.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,23 @@ fi
# We lock to version 3.44.0 to avoid https://github.com/y-scope/clp/issues/1352
readonly required_version="3.44.0"

package_preinstalled=0
if ! command -v task >/dev/null 2>&1; then
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"

go_task_bin="$(command -v task 2>/dev/null || true)"
if [ -n "${go_task_bin}" ]; then
package_preinstalled=0
echo "Preinstalled Task found at: ${go_task_bin}"
else
package_preinstalled=1
pipx install --force "go-task-bin==${required_version}"
pipx ensurepath
go_task_bin=$("${script_dir}/find-pipx-bin.sh" go-task-bin task)
echo "Pipx Task installed at: ${go_task_bin}"
fi
Comment on lines +25 to 27
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate resolved Task binary after pipx install.

Harden against a missing/incorrectly resolved path before using it.

-    go_task_bin=$("${script_dir}/find-pipx-bin.sh" go-task-bin task)
-    echo "Pipx Task installed at: ${go_task_bin}"
+    go_task_bin=$("${script_dir}/find-pipx-bin.sh" go-task-bin task)
+    if [ -z "${go_task_bin}" ] || [ ! -x "${go_task_bin}" ]; then
+        echo "Error: could not resolve Task binary after pipx install." >&2
+        echo "       Try: pipx uninstall go-task-bin && pipx install 'go-task-bin==${required_version}'" >&2
+        exit 1
+    fi
+    echo "Pipx Task installed at: ${go_task_bin}"
🤖 Prompt for AI Agents 
In components/core/tools/scripts/lib_install/pipx-packages/install-go-task.sh
around lines 25 to 27, the script echoes the resolved go_task_bin without
verifying it; validate that the variable is non-empty and points to an
executable file before using it, and if validation fails print a clear error
message to stderr and exit with a non-zero status so the failure is caught by
callers; if it passes, continue to echo the install location as currently done.


script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
task_version=$(task --silent --taskfile "${script_dir}/print-go-task-version.yaml")
if [[ "${task_version}" != "${required_version}" ]]; then
echo "Error: Task version ${task_version} is currently unsupported (must be" \
installed_version=$(${go_task_bin} --silent --taskfile "${script_dir}/print-go-task-version.yaml")
if [[ "${installed_version}" != "${required_version}" ]]; then
echo "Error: Task version ${installed_version} is currently unsupported (must be" \
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Quote the resolved binary when invoking.

Prevents breakage if the path contains spaces.

-installed_version=$(${go_task_bin} --silent --taskfile "${script_dir}/print-go-task-version.yaml")
+installed_version=$("${go_task_bin}" --silent --taskfile "${script_dir}/print-go-task-version.yaml")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
installed_version=$(${go_task_bin} --silent --taskfile "${script_dir}/print-go-task-version.yaml")
if [[ "${installed_version}" != "${required_version}" ]]; then
echo "Error: Task version ${installed_version} is currently unsupported (must be" \
installed_version=$("${go_task_bin}" --silent --taskfile "${script_dir}/print-go-task-version.yaml")
if [[ "${installed_version}" != "${required_version}" ]]; then
echo "Error: Task version ${installed_version} is currently unsupported (must be"
🤖 Prompt for AI Agents 
In components/core/tools/scripts/lib_install/pipx-packages/install-go-task.sh
around lines 29 to 31, the command substitution invokes the resolved binary
unquoted which breaks if the path contains spaces; update the invocation to
quote the binary (replace ${go_task_bin} with "${go_task_bin}") in the command
substitution (and any other places you call the binary) so the resolved path
with spaces is treated as a single argument.

"${required_version})."

if ((0 == "${package_preinstalled}")); then
Expand All @@ -35,3 +41,5 @@ if [[ "${task_version}" != "${required_version}" ]]; then

exit 1
fi

echo "Task version ${installed_version} installed at ${go_task_bin} satisfies version requirements."
14 changes: 11 additions & 3 deletions components/core/tools/scripts/lib_install/pipx-packages/install-uv.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,20 @@ readonly required_version_major_min=0
readonly required_version_minor_min=8
readonly required_version_min="${required_version_major_min}.${required_version_minor_min}"

package_preinstalled=0
if ! command -v uv >/dev/null 2>&1; then
uv_bin="$(command -v uv 2>/dev/null || true)"
if [ -n "${uv_bin}" ]; then
package_preinstalled=0
echo "Preinstalled uv found at: ${uv_bin}"
else
package_preinstalled=1
pipx install --force "uv>=${required_version_min}"
pipx ensurepath
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
uv_bin=$("${script_dir}/find-pipx-bin.sh" uv uv)
echo "Pipx uv installed at: ${uv_bin}"
fi
Comment on lines +24 to 27
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Validate resolved uv binary after pipx install.

Avoids opaque failures when uv_bin isn’t found.

-    script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
-    uv_bin=$("${script_dir}/find-pipx-bin.sh" uv uv)
-    echo "Pipx uv installed at: ${uv_bin}"
+    script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
+    uv_bin=$("${script_dir}/find-pipx-bin.sh" uv uv)
+    if [ -z "${uv_bin}" ] || [ ! -x "${uv_bin}" ]; then
+        echo "Error: could not resolve uv binary after pipx install." >&2
+        echo "       Try: pipx uninstall uv && pipx install 'uv>=${required_version_min}'" >&2
+        exit 1
+    fi
+    echo "Pipx uv installed at: ${uv_bin}"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
uv_bin=$("${script_dir}/find-pipx-bin.sh" uv uv)
echo "Pipx uv installed at: ${uv_bin}"
fi
script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" &>/dev/null && pwd)"
uv_bin=$("${script_dir}/find-pipx-bin.sh" uv uv)
if [ -z "${uv_bin}" ] || [ ! -x "${uv_bin}" ]; then
echo "Error: could not resolve uv binary after pipx install." >&2
echo " Try: pipx uninstall uv && pipx install 'uv>=${required_version_min}'" >&2
exit 1
fi
echo "Pipx uv installed at: ${uv_bin}"
fi
🤖 Prompt for AI Agents 
In components/core/tools/scripts/lib_install/pipx-packages/install-uv.sh around
lines 24 to 27, the script echoes the resolved uv binary but does not validate
it; update the script to check that uv_bin is non-empty and points to an
executable (e.g., test -n "$uv_bin" && [ -x "$uv_bin" ]) and if the check fails,
print a clear error message including the attempted path and exit with a
non-zero status so downstream steps fail fast and provide actionable
diagnostics.


installed_version=$(uv self version --output-format json | jq --raw-output ".version")
installed_version=$(${uv_bin} self version --output-format json | jq --raw-output ".version")
IFS=. read -r installd_version_major installed_version_minor _ <<<"${installed_version}"

if (("${installd_version_major}" == "${required_version_major_min}" && \
Expand All @@ -38,3 +44,5 @@ if (("${installd_version_major}" == "${required_version_major_min}" && \

exit 1
fi

echo "uv version ${installed_version} installed at ${uv_bin} satisfies version requirements."
Loading