From f3e145c95c3bcdab5077ace037fa926b0c1640d1 Mon Sep 17 00:00:00 2001 From: Tim Hitchener Date: Wed, 19 Oct 2022 12:22:32 -0400 Subject: [PATCH] Fix cyclic symlink on rebuild when filesystem caching is enabled (#408) * Checks for existing node_modules symlink * Fix unit tests; use Lstat instead of Readlink * Fix integration tests; create symlink when only launch_modules required --- build.go | 17 +++++++++++------ build_test.go | 22 ++++++++++++++++++++-- install_process.go | 20 +++++++++++++++++++- install_process_test.go | 4 ++-- 4 files changed, 52 insertions(+), 11 deletions(-) diff --git a/build.go b/build.go index bcc41735..3deb01de 100644 --- a/build.go +++ b/build.go @@ -115,7 +115,7 @@ func Build(pathParser PathParser, return packit.BuildResult{}, err } - currentModLayer, err = installProcess.SetupModules(context.WorkingDir, currentModLayer, layer.Path) + currentModLayer, err = installProcess.SetupModules(projectPath, currentModLayer, layer.Path) if err != nil { return packit.BuildResult{}, err } @@ -206,7 +206,7 @@ func Build(pathParser PathParser, return packit.BuildResult{}, err } - _, err = installProcess.SetupModules(context.WorkingDir, currentModLayer, layer.Path) + _, err = installProcess.SetupModules(projectPath, currentModLayer, layer.Path) if err != nil { return packit.BuildResult{}, err } @@ -221,6 +221,13 @@ func Build(pathParser PathParser, logger.Action("Completed in %s", duration.Round(time.Millisecond)) logger.Break() + if !build { + err = ensureNodeModulesSymlink(projectPath, layer.Path, tmpDir) + if err != nil { + return packit.BuildResult{}, err + } + } + layer.Metadata = map[string]interface{}{ "cache_sha": sha, } @@ -256,10 +263,7 @@ func Build(pathParser PathParser, } layer.ExecD = []string{filepath.Join(context.CNBPath, "bin", "setup-symlinks")} - err = ensureNodeModulesSymlink(projectPath, layer.Path, tmpDir) - if err != nil { - return packit.BuildResult{}, err - } + } else { logger.Process("Reusing cached layer %s", layer.Path) if !build { @@ -273,6 +277,7 @@ func Build(pathParser PathParser, layer.Launch = true layers = append(layers, layer) + } err = symlinker.Unlink(filepath.Join(homeDir, ".npmrc")) diff --git a/build_test.go b/build_test.go index d46a3983..4e16d874 100644 --- a/build_test.go +++ b/build_test.go @@ -211,7 +211,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(installProcess.ShouldRunCall.Receives.WorkingDir).To(Equal(filepath.Join(workingDir, "some-project-dir"))) - Expect(installProcess.SetupModulesCall.Receives.WorkingDir).To(Equal(workingDir)) + Expect(installProcess.SetupModulesCall.Receives.WorkingDir).To(Equal(filepath.Join(workingDir, "some-project-dir"))) Expect(installProcess.SetupModulesCall.Receives.CurrentModulesLayerPath).To(Equal("")) Expect(installProcess.SetupModulesCall.Receives.NextModulesLayerPath).To(Equal(layer.Path)) @@ -265,6 +265,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { "PATH.delim": ":", })) Expect(layer.Launch).To(BeTrue()) + Expect(layer.Build).To(BeFalse()) Expect(layer.Metadata).To(Equal( map[string]interface{}{ "cache_sha": "some-awesome-shasum", @@ -300,7 +301,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(installProcess.ShouldRunCall.Receives.WorkingDir).To(Equal(filepath.Join(workingDir, "some-project-dir"))) - Expect(installProcess.SetupModulesCall.Receives.WorkingDir).To(Equal(workingDir)) + Expect(installProcess.SetupModulesCall.Receives.WorkingDir).To(Equal(filepath.Join(workingDir, "some-project-dir"))) Expect(installProcess.SetupModulesCall.Receives.CurrentModulesLayerPath).To(Equal("")) Expect(installProcess.SetupModulesCall.Receives.NextModulesLayerPath).To(Equal(layer.Path)) @@ -309,6 +310,14 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(installProcess.ExecuteCall.Receives.Launch).To(BeTrue()) Expect(sbomGenerator.GenerateCall.Receives.Dir).To(Equal(workingDir)) + + workspaceLink, err := os.Readlink(filepath.Join(workingDir, "some-project-dir", "node_modules")) + Expect(err).NotTo(HaveOccurred()) + Expect(workspaceLink).To(Equal(filepath.Join(tmpDir, "node_modules"))) + + tmpLink, err := os.Readlink(filepath.Join(tmpDir, "node_modules")) + Expect(err).NotTo(HaveOccurred()) + Expect(tmpLink).To(Equal(filepath.Join(layersDir, "launch-modules", "node_modules"))) }) }) @@ -356,6 +365,7 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { it.Before(func() { entryResolver.MergeLayerTypesCall.Returns.Launch = true entryResolver.MergeLayerTypesCall.Returns.Build = true + pathParser.GetCall.Returns.ProjectPath = workingDir installProcess.SetupModulesCall.Stub = func(w string, c string, n string) (string, error) { setupModulesCalls = append(setupModulesCalls, setupModulesParams{ @@ -406,6 +416,14 @@ func testBuild(t *testing.T, context spec.G, it spec.S) { Expect(setupModulesCalls[1].WorkingDir).To(Equal(workingDir)) Expect(setupModulesCalls[1].CurrentModulesLayerPath).To(Equal(result.Layers[0].Path)) Expect(setupModulesCalls[1].NextModulesLayerPath).To(Equal(result.Layers[1].Path)) + + workspaceLink, err := os.Readlink(filepath.Join(workingDir, "node_modules")) + Expect(err).NotTo(HaveOccurred()) + Expect(workspaceLink).To(Equal(filepath.Join(tmpDir, "node_modules"))) + + tmpLink, err := os.Readlink(filepath.Join(tmpDir, "node_modules")) + Expect(err).NotTo(HaveOccurred()) + Expect(tmpLink).To(Equal(filepath.Join(layersDir, "build-modules", "node_modules"))) }) }) diff --git a/install_process.go b/install_process.go index 3f97d282..d8d11cd4 100644 --- a/install_process.go +++ b/install_process.go @@ -99,8 +99,26 @@ func (ip YarnInstallProcess) SetupModules(workingDir, currentModulesLayerPath, n } } else { - err := os.MkdirAll(filepath.Join(workingDir, "node_modules"), os.ModePerm) + + file, err := os.Lstat(filepath.Join(workingDir, "node_modules")) + if err != nil { + if !errors.Is(err, os.ErrNotExist) { + return "", fmt.Errorf("failed to stat node_modules directory: %w", err) + } + + } + + if file != nil && file.Mode()&os.ModeSymlink == os.ModeSymlink { + err = os.RemoveAll(filepath.Join(workingDir, "node_modules")) + if err != nil { + //not tested + return "", fmt.Errorf("failed to remove node_modules symlink: %w", err) + } + } + + err = os.MkdirAll(filepath.Join(workingDir, "node_modules"), os.ModePerm) if err != nil { + //not directly tested return "", fmt.Errorf("failed to create node_modules directory: %w", err) } diff --git a/install_process_test.go b/install_process_test.go index fbc30320..7a094de8 100644 --- a/install_process_test.go +++ b/install_process_test.go @@ -256,7 +256,7 @@ func testInstallProcess(t *testing.T, context spec.G, it spec.S) { }) }) - context("node_modules directory cannot be created in working directory", func() { + context("Lstat() cannot be run on node_modules in working directory", func() { it.Before(func() { Expect(os.Chmod(workingDir, 0000)).To(Succeed()) }) @@ -267,7 +267,7 @@ func testInstallProcess(t *testing.T, context spec.G, it spec.S) { it("returns an error", func() { _, err := installProcess.SetupModules(workingDir, "", nextModulesLayerPath) - Expect(err).To(MatchError(ContainSubstring("failed to create node_modules directory:"))) + Expect(err).To(MatchError(ContainSubstring("failed to stat node_modules directory:"))) Expect(err).To(MatchError(ContainSubstring("permission denied"))) }) })