Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/dev' into fix_java_build_times…
Browse files Browse the repository at this point in the history
…tamp
  • Loading branch information
attiasas committed Nov 5, 2023
2 parents 4f03b7e + 13f94ab commit ee29567
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 6 deletions.
26 changes: 24 additions & 2 deletions build/utils/npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"golang.org/x/exp/slices"
"os"
"os/exec"
"path/filepath"
Expand All @@ -16,6 +17,8 @@ import (
"github.com/jfrog/gofrog/version"
)

const npmInstallCommand = "install"

// CalculateNpmDependenciesList gets an npm project's dependencies.
func CalculateNpmDependenciesList(executablePath, srcPath, moduleId string, npmParams NpmTreeDepListParam, calculateChecksums bool, log utils.Log) ([]entities.Dependency, error) {
if log == nil {
Expand Down Expand Up @@ -136,7 +139,7 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np
return nil, isDirExistsErr
}
if !isPackageLockExist || (npmListParams.OverwritePackageLock && checkIfLockFileShouldBeUpdated(srcPath, log)) {
err := installPackageLock(executablePath, srcPath, npmListParams.Args, log, npmVersion)
err := installPackageLock(executablePath, srcPath, npmListParams.InstallCommandArgs, npmListParams.Args, log, npmVersion)
if err != nil {
return nil, err
}
Expand All @@ -151,9 +154,11 @@ func runNpmLsWithoutNodeModules(executablePath, srcPath string, npmListParams Np
return data, nil
}

func installPackageLock(executablePath, srcPath string, npmArgs []string, log utils.Log, npmVersion *version.Version) error {
func installPackageLock(executablePath, srcPath string, npmInstallCommandArgs, npmArgs []string, log utils.Log, npmVersion *version.Version) error {
if npmVersion.AtLeast("6.0.0") {
npmArgs = append(npmArgs, "--package-lock-only")
// Including any 'install' command flags that were supplied by the user in preceding steps of the process, while ensuring that duplicates are avoided.
npmArgs = append(npmArgs, filterUniqueArgs(npmInstallCommandArgs, npmArgs)...)
// Installing package-lock to generate the dependencies map.
_, _, err := RunNpmCmd(executablePath, srcPath, AppendNpmCommand(npmArgs, "install"), log)
if err != nil {
Expand All @@ -164,6 +169,20 @@ func installPackageLock(executablePath, srcPath string, npmArgs []string, log ut
return errors.New("it looks like you’re using version " + npmVersion.GetVersion() + " of the npm client. Versions below 6.0.0 require running `npm install` before running this command")
}

// Removes any arguments from argsToFilter that are already present in existingArgs. Furthermore, excludes the "install" command and retains only the flags in the resulting argument list.
func filterUniqueArgs(argsToFilter []string, existingArgs []string) []string {
var filteredArgs []string
for _, arg := range argsToFilter {
if arg == npmInstallCommand {
continue
}
if !slices.Contains(existingArgs, arg) {
filteredArgs = append(filteredArgs, arg)
}
}
return filteredArgs
}

// Check if package.json has been modified.
// This might indicate the addition of new packages to package.json that haven't been reflected in package-lock.json.
func checkIfLockFileShouldBeUpdated(srcPath string, log utils.Log) bool {
Expand Down Expand Up @@ -192,7 +211,10 @@ func GetNpmVersion(executablePath string, log utils.Log) (*version.Version, erro
}

type NpmTreeDepListParam struct {
// Required for the 'install' and 'ls' commands that could be triggered during the construction of the NPM dependency tree
Args []string
// Optional user-supplied arguments for the 'install' command. These arguments are not available from all entry points. They may be employed when constructing the NPM dependency tree, which could necessitate the execution of 'npm install...'
InstallCommandArgs []string
// Ignore the node_modules folder if exists, using the '--package-lock-only' flag
IgnoreNodeModules bool
// Rewrite package-lock.json, if exists.
Expand Down
41 changes: 37 additions & 4 deletions build/utils/npm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func TestReadPackageInfo(t *testing.T) {
return
}

tests := []struct {
testcases := []struct {
json string
pi *PackageInfo
}{
Expand All @@ -35,7 +35,7 @@ func TestReadPackageInfo(t *testing.T) {
&PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: "@jfrog"}},
{`{}`, &PackageInfo{}},
}
for _, test := range tests {
for _, test := range testcases {
t.Run(test.json, func(t *testing.T) {
packInfo, err := ReadPackageInfo([]byte(test.json), npmVersion)
assert.NoError(t, err)
Expand Down Expand Up @@ -94,14 +94,14 @@ func TestReadPackageInfoFromPackageJsonIfExistErr(t *testing.T) {
}

func TestGetDeployPath(t *testing.T) {
tests := []struct {
testcases := []struct {
expectedPath string
pi *PackageInfo
}{
{`build-info-go-tests/-/build-info-go-tests-1.0.0.tgz`, &PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: ""}},
{`@jfrog/build-info-go-tests/-/build-info-go-tests-1.0.0.tgz`, &PackageInfo{Name: "build-info-go-tests", Version: "1.0.0", Scope: "@jfrog"}},
}
for _, test := range tests {
for _, test := range testcases {
t.Run(test.expectedPath, func(t *testing.T) {
assert.Equal(t, test.expectedPath, test.pi.GetDeployPath())
})
Expand Down Expand Up @@ -430,3 +430,36 @@ func validateDependencies(t *testing.T, projectPath string, npmArgs []string) {
// Asserting there is at least one dependency.
assert.Greater(t, len(dependencies), 0, "Error: dependencies are not found!")
}

func TestFilterUniqueArgs(t *testing.T) {
var testcases = []struct {
argsToFilter []string
alreadyExists []string
expectedResult []string
}{
{
argsToFilter: []string{"install"},
alreadyExists: []string{},
expectedResult: nil,
},
{
argsToFilter: []string{"install", "--flagA"},
alreadyExists: []string{"--flagA"},
expectedResult: nil,
},
{
argsToFilter: []string{"install", "--flagA", "--flagB"},
alreadyExists: []string{"--flagA"},
expectedResult: []string{"--flagB"},
},
{
argsToFilter: []string{"install", "--flagA", "--flagB"},
alreadyExists: []string{"--flagA", "--flagC"},
expectedResult: []string{"--flagB"},
},
}

for _, testcase := range testcases {
assert.Equal(t, testcase.expectedResult, filterUniqueArgs(testcase.argsToFilter, testcase.alreadyExists))
}
}

0 comments on commit ee29567

Please sign in to comment.