From 52ff32c4d8ebeb887ecdf6e918c4d550e4daf7cf Mon Sep 17 00:00:00 2001 From: Michael Sverdlov Date: Sun, 20 Aug 2023 19:58:57 +0300 Subject: [PATCH] Fix npm commands missing standard output (#181) --- build/npm.go | 8 +++++--- build/utils/npm.go | 42 ++++++++++++++++++++--------------------- build/utils/npm_test.go | 32 ++++++++++++++++--------------- go.mod | 4 ++-- go.sum | 8 ++++---- 5 files changed, 49 insertions(+), 45 deletions(-) diff --git a/build/npm.go b/build/npm.go index 45659e0e..66d52e04 100644 --- a/build/npm.go +++ b/build/npm.go @@ -54,10 +54,12 @@ func newNpmModule(srcPath string, containingBuild *Build) (*NpmModule, error) { func (nm *NpmModule) Build() error { if len(nm.npmArgs) > 0 { - _, errData, err := buildutils.RunNpmCmd(nm.executablePath, nm.srcPath, nm.npmArgs, &utils.NullLog{}) + output, _, err := buildutils.RunNpmCmd(nm.executablePath, nm.srcPath, nm.npmArgs, &utils.NullLog{}) + if len(output) > 0 { + nm.containingBuild.logger.Output(strings.TrimSpace(string(output))) + } if err != nil { - // NpmArgs[0] includes the npm command. - return errors.New("couldn't run npm " + nm.npmArgs[0] + ": " + string(errData)) + return err } // After executing the user-provided command, cleaning npmArgs is needed. nm.filterNpmArgsFlags() diff --git a/build/utils/npm.go b/build/utils/npm.go index 1be6a632..114783ef 100644 --- a/build/utils/npm.go +++ b/build/utils/npm.go @@ -123,10 +123,9 @@ func runNpmLsWithNodeModules(executablePath, srcPath string, npmArgs []string, l data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "ls"), log) if err != nil { // It is optional for the function to return this error. - log.Warn("npm list command failed with error:", err.Error()) - } - if len(errData) > 0 { - log.Warn("Some errors occurred while collecting dependencies info:\n" + string(errData)) + log.Warn(err.Error()) + } else if len(errData) > 0 { + log.Warn("Encountered some issues while running 'npm ls' command:\n" + strings.TrimSpace(string(errData))) } return } @@ -145,10 +144,9 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmArgs []string npmArgs = append(npmArgs, "--json", "--all", "--long", "--package-lock-only") data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "ls"), log) if err != nil { - log.Warn("npm list command failed with error:", err.Error()) - } - if len(errData) > 0 { - log.Warn("Some errors occurred while collecting dependencies info:\n" + string(errData)) + log.Warn(err.Error()) + } else if len(errData) > 0 { + log.Warn("Encountered some issues while running 'npm ls' command:\n" + strings.TrimSpace(string(errData))) } return data, nil } @@ -157,9 +155,9 @@ func installPackageLock(executablePath, srcPath string, npmArgs []string, log ut if npmVersion.AtLeast("6.0.0") { npmArgs = append(npmArgs, "--package-lock-only") // Installing package-lock to generate the dependencies map. - _, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log) + _, _, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log) if err != nil { - return errors.New("Some errors occurred while installing package-lock: " + string(errData)) + return err } return nil } @@ -354,14 +352,14 @@ func appendScopes(oldScopes []string, newScopes []string) []string { func RunNpmCmd(executablePath, srcPath string, npmArgs []string, log utils.Log) (stdResult, errResult []byte, err error) { log.Debug("Running npm " + npmArgs[0] + " command.") - tmpArgs := make([]string, 0) + args := make([]string, 0) for i := 0; i < len(npmArgs); i++ { if strings.TrimSpace(npmArgs[i]) != "" { - tmpArgs = append(tmpArgs, npmArgs[i]) + args = append(args, npmArgs[i]) } } - command := exec.Command(executablePath, tmpArgs...) + command := exec.Command(executablePath, args...) command.Dir = srcPath outBuffer := bytes.NewBuffer([]byte{}) command.Stdout = outBuffer @@ -371,11 +369,10 @@ func RunNpmCmd(executablePath, srcPath string, npmArgs []string, log utils.Log) errResult = errBuffer.Bytes() stdResult = outBuffer.Bytes() if err != nil { - err = errors.New("error while running the command :'" + executablePath + " " + strings.Join(tmpArgs, " ") + "'\nError output is:\n" + string(errResult) + "\nCommand error: is:\n" + err.Error()) + err = fmt.Errorf("error while running '%s %s': %s\n%s", executablePath, strings.Join(args, " "), strings.TrimSpace(string(errResult)), err.Error()) return } - // The npm command is the first element in tmpArgs array. - log.Debug("npm " + tmpArgs[0] + " standard output is:\n" + string(stdResult)) + log.Debug("npm '" + strings.Join(args, " ") + "' standard output is:\n" + strings.TrimSpace(string(stdResult))) return } @@ -441,6 +438,9 @@ func ReadPackageInfo(data []byte, npmVersion *version.Version) (*PackageInfo, er } func (pi *PackageInfo) BuildInfoModuleId() string { + if pi.Name == "" || pi.Version == "" { + return "" + } nameBase := fmt.Sprintf("%s:%s", pi.Name, pi.Version) if pi.Scope == "" { return nameBase @@ -450,6 +450,7 @@ func (pi *PackageInfo) BuildInfoModuleId() string { func (pi *PackageInfo) GetDeployPath() string { fileName := fmt.Sprintf("%s-%s.tgz", pi.Name, pi.Version) + // The hyphen part below "/-/" is there in order to follow the layout used by the public NPM registry. if pi.Scope == "" { return fmt.Sprintf("%s/-/%s", pi.Name, fileName) } @@ -482,12 +483,11 @@ func removeVersionPrefixes(packageInfo *PackageInfo) { func GetNpmConfigCache(srcPath, executablePath string, npmArgs []string, log utils.Log) (string, error) { npmArgs = append([]string{"get", "cache"}, npmArgs...) data, errData, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(append(npmArgs, "--json=false"), "config"), log) - // Some warnings and messages of npm are printed to stderr. They don't cause the command to fail, but we'd want to show them to the user. - if len(errData) > 0 { - log.Warn("error while running the command :'" + executablePath + " " + strings.Join(npmArgs, " ") + ":\n" + string(errData)) - } if err != nil { - return "", fmt.Errorf("'%s %s' npm config command failed with an error: %s", executablePath, strings.Join(npmArgs, " "), err.Error()) + return "", err + } else if len(errData) > 0 { + // Some warnings and messages of npm are printed to stderr. They don't cause the command to fail, but we'd want to show them to the user. + log.Warn("Encountered some issues while running 'npm get cache' command:\n" + string(errData)) } cachePath := filepath.Join(strings.Trim(string(data), "\n"), "_cacache") found, err := utils.IsDirExists(cachePath, true) diff --git a/build/utils/npm_test.go b/build/utils/npm_test.go index b0bb6e81..fc995b30 100644 --- a/build/utils/npm_test.go +++ b/build/utils/npm_test.go @@ -199,7 +199,7 @@ func TestDependencyWithNoIntegrity(t *testing.T) { } // A project built differently for each operating system. -func TestDependenciesTreeDiffrentBetweenOss(t *testing.T) { +func TestDependenciesTreeDifferentBetweenOKs(t *testing.T) { npmVersion, _, err := GetNpmVersionAndExecPath(logger) assert.NoError(t, err) path, err := filepath.Abs(filepath.Join("..", "testdata")) @@ -242,19 +242,21 @@ func TestNpmProdFlag(t *testing.T) { {"--prod", 1}, } for _, entry := range testDependencyScopes { - projectPath, cleanup := testdatautils.CreateNpmTest(t, path, "project3", false, npmVersion) - defer cleanup() - cacachePath := filepath.Join(projectPath, "tmpcache") - npmArgs := []string{"--cache=" + cacachePath, entry.scope} - - // Install dependencies in the npm project. - _, _, err = RunNpmCmd("npm", projectPath, AppendNpmCommand(npmArgs, "ci"), logger) - assert.NoError(t, err) - - // Calculate dependencies with scope. - dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) - assert.NoError(t, err) - assert.Len(t, dependencies, entry.totalDeps) + func() { + projectPath, cleanup := testdatautils.CreateNpmTest(t, path, "project3", false, npmVersion) + defer cleanup() + cacachePath := filepath.Join(projectPath, "tmpcache") + npmArgs := []string{"--cache=" + cacachePath, entry.scope} + + // Install dependencies in the npm project. + _, _, err = RunNpmCmd("npm", projectPath, AppendNpmCommand(npmArgs, "ci"), logger) + assert.NoError(t, err) + + // Calculate dependencies with scope. + dependencies, err := CalculateNpmDependenciesList("npm", projectPath, "build-info-go-tests", npmArgs, true, logger) + assert.NoError(t, err) + assert.Len(t, dependencies, entry.totalDeps) + }() } } @@ -296,7 +298,7 @@ func TestGetConfigCacheNpmIntegration(t *testing.T) { // 2. node_module doesn't exist in the project and generating dependencies needs package-lock. func validateDependencies(t *testing.T, projectPath string, npmArgs []string) { // Install dependencies in the npm project. - _, _, err := RunNpmCmd("npm", projectPath, AppendNpmCommand(npmArgs,"ci"), logger) + _, _, err := RunNpmCmd("npm", projectPath, AppendNpmCommand(npmArgs, "ci"), logger) assert.NoError(t, err) // Calculate dependencies. diff --git a/go.mod b/go.mod index c2c36f8c..4848b166 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/stretchr/testify v1.8.4 github.com/urfave/cli/v2 v2.25.7 github.com/xeipuuv/gojsonschema v1.2.0 - golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b + golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 ) require ( @@ -23,6 +23,6 @@ require ( github.com/xeipuuv/gojsonpointer v0.0.0-20180127040702-4e3ac2762d5f // indirect github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect - golang.org/x/sys v0.1.0 // indirect + golang.org/x/sys v0.11.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 65cd60bd..0f1e7399 100644 --- a/go.sum +++ b/go.sum @@ -34,11 +34,11 @@ github.com/xeipuuv/gojsonschema v1.2.0 h1:LhYJRs+L4fBtjZUfuSZIKGeVu0QRy8e5Xi7D17 github.com/xeipuuv/gojsonschema v1.2.0/go.mod h1:anYRn/JVcOK2ZgGU+IjEV4nwlhoK5sQluxsYJ78Id3Y= github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU= github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673/go.mod h1:N3UwUGtsrSj3ccvlPHLoLsHnpR27oXr4ZE984MbSER8= -golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b h1:r+vk0EmXNmekl0S0BascoeeoHk/L7wmaW2QF90K+kYI= -golang.org/x/exp v0.0.0-20230801115018-d63ba01acd4b/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc= +golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63 h1:m64FZMko/V45gv0bNmrNYoDEq8U5YUhetc9cBWKS1TQ= +golang.org/x/exp v0.0.0-20230817173708-d852ddb80c63/go.mod h1:0v4NqG35kSWCMzLaMeX+IQrlSnVE/bqGSyC2cz/9Le8= golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U= -golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.11.0 h1:eG7RXZHdqOJ1i+0lgLgCpSXAp6M3LYlAo6osgSi0xOM= +golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=