Skip to content

Commit 10fdf16

Browse files
gtsiolisgtsiolis
committed
Fix npm update failing for version manager installs
1 parent 4405213 commit 10fdf16

File tree

5 files changed

+21
-88
lines changed

5 files changed

+21
-88
lines changed

‎internal/update/install_method.go‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -64,26 +64,3 @@ func classifyPath(resolved string) InstallMethod {
6464

6565
return InstallBinary
6666
}
67-
68-
// npmProjectDir returns the project directory for a local npm install,
69-
// or empty string for a global install. A local install has a package.json
70-
// in the parent of the node_modules directory.
71-
func npmProjectDir(resolvedPath string) string {
72-
// Walk up to find node_modules, then check for package.json one level above
73-
dir := resolvedPath
74-
for {
75-
parent := filepath.Dir(dir)
76-
if parent == dir {
77-
break
78-
}
79-
if filepath.Base(dir) == "node_modules" {
80-
pkgJSON := filepath.Join(parent, "package.json")
81-
if _, err := os.Stat(pkgJSON); err == nil {
82-
return parent
83-
}
84-
return ""
85-
}
86-
dir = parent
87-
}
88-
return ""
89-
}

‎internal/update/install_method_test.go‎

Lines changed: 5 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package update
22

33
import (
4-
"os"
5-
"path/filepath"
64
"testing"
75
)
86

@@ -34,6 +32,11 @@ func TestClassifyPath(t *testing.T) {
3432
path: "/usr/local/lib/node_modules/@localstack/lstk_darwin_amd64/lstk",
3533
want: InstallNPM,
3634
},
35+
{
36+
name: "npm global install via asdf",
37+
path: "/Users/geo/.asdf/installs/nodejs/22.12.0/lib/node_modules/@localstack/lstk_darwin_arm64/lstk",
38+
want: InstallNPM,
39+
},
3740
{
3841
name: "standalone binary in usr local bin",
3942
path: "/usr/local/bin/lstk",
@@ -61,46 +64,3 @@ func TestClassifyPath(t *testing.T) {
6164
})
6265
}
6366
}
64-
65-
func TestNpmProjectDir(t *testing.T) {
66-
t.Parallel()
67-
68-
t.Run("local install with package.json", func(t *testing.T) {
69-
t.Parallel()
70-
dir := t.TempDir()
71-
if err := os.WriteFile(filepath.Join(dir, "package.json"), []byte("{}"), 0o644); err != nil {
72-
t.Fatal(err)
73-
}
74-
binaryPath := filepath.Join(dir, "node_modules", "@localstack", "lstk_darwin_arm64", "lstk")
75-
if err := os.MkdirAll(filepath.Dir(binaryPath), 0o755); err != nil {
76-
t.Fatal(err)
77-
}
78-
79-
got := npmProjectDir(binaryPath)
80-
if got != dir {
81-
t.Fatalf("npmProjectDir() = %q, want %q", got, dir)
82-
}
83-
})
84-
85-
t.Run("global install without package.json", func(t *testing.T) {
86-
t.Parallel()
87-
dir := t.TempDir()
88-
binaryPath := filepath.Join(dir, "lib", "node_modules", "@localstack", "lstk_darwin_arm64", "lstk")
89-
if err := os.MkdirAll(filepath.Dir(binaryPath), 0o755); err != nil {
90-
t.Fatal(err)
91-
}
92-
93-
got := npmProjectDir(binaryPath)
94-
if got != "" {
95-
t.Fatalf("npmProjectDir() = %q, want empty string", got)
96-
}
97-
})
98-
99-
t.Run("non-npm path", func(t *testing.T) {
100-
t.Parallel()
101-
got := npmProjectDir("/usr/local/bin/lstk")
102-
if got != "" {
103-
t.Fatalf("npmProjectDir() = %q, want empty string", got)
104-
}
105-
})
106-
}

‎internal/update/npm.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,8 @@ import (
77
"github.com/localstack/lstk/internal/output"
88
)
99

10-
func updateNPM(ctx context.Context, sink output.Sink, projectDir string) error {
11-
var cmd *exec.Cmd
12-
if projectDir != "" {
13-
cmd = exec.CommandContext(ctx, "npm", "install", "@localstack/lstk@latest")
14-
cmd.Dir = projectDir
15-
} else {
16-
cmd = exec.CommandContext(ctx, "npm", "install", "-g", "@localstack/lstk@latest")
17-
}
10+
func updateNPM(ctx context.Context, sink output.Sink) error {
11+
cmd := exec.CommandContext(ctx, "npm", "install", "-g", "@localstack/lstk@latest")
1812
w := newLogLineWriter(sink, output.LogSourceNPM)
1913
cmd.Stdout = w
2014
cmd.Stderr = w

‎internal/update/update.go‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,8 @@ func applyUpdate(ctx context.Context, sink output.Sink, latest, githubToken stri
6363
output.EmitNote(sink, "Installed through Homebrew, running brew upgrade")
6464
err = updateHomebrew(ctx, sink)
6565
case InstallNPM:
66-
projectDir := npmProjectDir(info.ResolvedPath)
67-
if projectDir != "" {
68-
output.EmitNote(sink, fmt.Sprintf("Installed through npm (local), running npm install in %s", projectDir))
69-
} else {
70-
output.EmitNote(sink, "Installed through npm (global), running npm install -g")
71-
}
72-
err = updateNPM(ctx, sink, projectDir)
66+
output.EmitNote(sink, "Installed through npm, running npm install -g")
67+
err = updateNPM(ctx, sink)
7368
default:
7469
output.EmitSpinnerStart(sink, "Downloading update")
7570
err = updateBinary(ctx, latest, githubToken)

‎test/integration/update_test.go‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,19 @@ func requireNPM(t *testing.T) {
4747
}
4848
}
4949

50-
func TestUpdateNPMLocalInstall(t *testing.T) {
50+
func TestUpdateNPMInstall(t *testing.T) {
5151
requireNPM(t)
5252

53+
// Skip if lstk is already installed globally (e.g., via Homebrew).
54+
// npm install -g fails with EEXIST when it tries to create a symlink
55+
// over an existing binary at the same path.
56+
if path, err := exec.LookPath("lstk"); err == nil {
57+
t.Skipf("lstk already installed at %s, would conflict with npm install -g", path)
58+
}
59+
5360
ctx := testContext(t)
5461

55-
// Set up a fake local npm project.
62+
// Set up a fake local npm project so we get a binary inside node_modules.
5663
// On Windows, t.TempDir() may return a short 8.3 path (e.g. RUNNER~1)
5764
// while the program resolves the long path. EvalSymlinks normalizes both.
5865
projectDir, err := filepath.EvalSymlinks(t.TempDir())
@@ -93,16 +100,16 @@ func TestUpdateNPMLocalInstall(t *testing.T) {
93100
out, err = buildCmd.CombinedOutput()
94101
require.NoError(t, err, "go build failed: %s", string(out))
95102

96-
// Run the binary directly (not through npx) so os.Executable() resolves to the node_modules path
103+
// Run the binary directly (not through npx) so os.Executable() resolves to the node_modules path.
104+
// The update should always use `npm install -g` regardless of local/global context.
97105
cmd := exec.CommandContext(ctx, nmBinaryPath, "update", "--non-interactive")
98106
cmd.Dir = projectDir
99107
stdout, err := cmd.CombinedOutput()
100108
stdoutStr := string(stdout)
101109

102110
require.NoError(t, err, "lstk update failed: %s", stdoutStr)
103111
requireExitCode(t, 0, err)
104-
assert.Contains(t, stdoutStr, "npm (local)", "should detect local npm install")
105-
assert.Contains(t, stdoutStr, projectDir, "should show the project directory")
112+
assert.Contains(t, stdoutStr, "npm install -g", "should always use global install")
106113
assert.Contains(t, stdoutStr, "Updated to", "should complete the update")
107114
}
108115

0 commit comments

Comments
 (0)