Skip to content
This repository was archived by the owner on May 15, 2025. It is now read-only.

fix(devcontainers-cli): add PNPM_HOME for pnpm package manager #433

Merged
merged 3 commits into from
Apr 18, 2025
Merged

fix(devcontainers-cli): add PNPM_HOME for pnpm package manager #433

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9a0af2c
add PNPM_HOME on pnpm
defelmnq Apr 17, 2025
1adf7aa
format
defelmnq Apr 17, 2025
5e147bb
update script with maf suggestions
defelmnq Apr 18, 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: 2 additions & 2 deletions devcontainers-cli/main.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const executeScriptInContainerWithPackageManager = async (
await execContainer(id, [
shell,
"-c",
"apk add nodejs npm && npm install -g pnpm",
`wget -qO- https://get.pnpm.io/install.sh | ENV="$HOME/.shrc" SHELL="$(which sh)" sh -`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Purely testing purpose, done to not rely on NPM.

]);
} else if (packageManager === "yarn") {
await execContainer(id, [
Expand Down Expand Up @@ -86,7 +86,7 @@ describe("devcontainers-cli", async () => {
expect(output.stdout[output.stdout.length - 1]).toEqual(
"🥳 @devcontainers/cli has been installed into /usr/local/bin/devcontainer!",
);
});
}, 15000);

it("installs devcontainers-cli with yarn", async () => {
const state = await runTerraformApply(import.meta.dir, {
Expand Down
17 changes: 12 additions & 5 deletions devcontainers-cli/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ if ! command -v docker > /dev/null 2>&1; then
fi

# Determine the package manager to use: npm, pnpm, or yarn
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the ordering to use PNPM as a last option, if NPM is installed too.

if command -v pnpm > /dev/null 2>&1; then
PACKAGE_MANAGER="pnpm"
elif command -v yarn > /dev/null 2>&1; then
if command -v yarn > /dev/null 2>&1; then
PACKAGE_MANAGER="yarn"
elif command -v npm > /dev/null 2>&1; then
PACKAGE_MANAGER="npm"
elif command -v pnpm > /dev/null 2>&1; then
PACKAGE_MANAGER="pnpm"
else
echo "ERROR: No supported package manager (npm, pnpm, yarn) is installed. Please install one first." 1>&2
exit 1
Expand All @@ -26,8 +26,15 @@ fi
echo "Installing @devcontainers/cli using $PACKAGE_MANAGER..."

# Install @devcontainers/cli using the selected package manager
if [ "$PACKAGE_MANAGER" = "npm" ] || [ "$PACKAGE_MANAGER" = "pnpm" ]; then
$PACKAGE_MANAGER install -g @devcontainers/cli \
if [ "$PACKAGE_MANAGER" = "npm" ]; then
$PACKAGE_MANAGER install -g @devcontainers/cli \
&& echo "🥳 @devcontainers/cli has been installed into $(which devcontainer)!"
elif [ "$PACKAGE_MANAGER" = "pnpm" ]; then
# if PNPM_HOME is not set, set it to the bin directory of the script
if [ -z "$PNPM_HOME" ]; then
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main part of the fix, adding $PNPM_HOME if not set yet.

export PNPM_HOME="$CODER_SCRIPT_BIN_DIR"
fi
$PACKAGE_MANAGER add -g @devcontainers/cli \
&& echo "🥳 @devcontainers/cli has been installed into $(which devcontainer)!"
elif [ "$PACKAGE_MANAGER" = "yarn" ]; then
$PACKAGE_MANAGER global add @devcontainers/cli \
Expand Down
Loading