Skip to content

Commit

Permalink
Fix an issue with parsing versions with yarn
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 655001508
Change-Id: I34a9709114e6cafd477d737e8162e36e1e480f76
  • Loading branch information
@abhis3 @copybara-github
abhis3 authored and copybara-github committed Jul 23, 2024
1 parent f2c57f1 commit f335a79
Show file tree
Hide file tree
Showing 10 changed files with 1,861 additions and 160 deletions.
82 changes: 44 additions & 38 deletions pkg/nodejs/nodejs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"io/ioutil"
"os"
"path/filepath"
"regexp"
"strings"

"github.com/GoogleCloudPlatform/buildpacks/pkg/cache"
Expand Down Expand Up @@ -53,9 +54,10 @@ const (
var semVer11 = semver.MustParse("11.0.0")

var (
cachedPackageJSONs = map[string]*PackageJSON{}
cachedPackageJSONs = map[string]*PackageJSON{}
possibleLockfileFilenames = []string{"pnpm-lock.yaml", "yarn.lock", "npm-shrinkwrap.json", "package-lock.json"}
dependencyRegex = regexp.MustCompile(`\r?\n\r?\n`)
)
var possibleLockfileFilenames = []string{"pnpm-lock.yaml", "yarn.lock", "npm-shrinkwrap.json", "package-lock.json"}

type packageEnginesJSON struct {
Node string `json:"node"`
Expand Down Expand Up @@ -117,9 +119,8 @@ type PnpmV9Lockfile struct {

// NodeDependencies represents the dependencies of a Node package via its package.json and lockfile.
type NodeDependencies struct {
PackageJSON *PackageJSON
LockfileName string
LockfileBytes []byte
PackageJSON *PackageJSON
LockfilePath string
}

// ReadPackageJSONIfExists returns deserialized package.json from the given dir. If the provided dir
Expand All @@ -146,6 +147,7 @@ func ReadPackageJSONIfExists(dir string) (*PackageJSON, error) {
// ReadNodeDependencies looks for a package.json and lockfile in either appDir or rootDir. The
// lockfile must either be in the same directory as package.json or be in the application root,
// otherwise an error is returned.
// TODO (b/354012293): In the future we should read the data into structs for easier manipulation.
func ReadNodeDependencies(ctx *gcp.Context, appDir string) (*NodeDependencies, error) {
rootDir := ctx.ApplicationRoot()
if !strings.HasPrefix(appDir, rootDir) {
Expand All @@ -172,38 +174,34 @@ func ReadNodeDependencies(ctx *gcp.Context, appDir string) (*NodeDependencies, e
}
}

// Try to read lockfile from the same dir; if there is no lockfile, check the application root for
// a lockfile. Return an error if no lockfile is found.
name, bytes, err := readLockfileBytesIfExists(dir)
if err != nil {
return nil, err
// Try to find a lockfile from the same dir, if there is none then check the application root.
// Return an error if no lockfile is found.
path, err := findValidLockfileInDir(dir)
if err == nil {
return &NodeDependencies{pjs, path}, nil
}
if bytes != nil {
return &NodeDependencies{pjs, name, bytes}, nil
}
name, bytes, err = readLockfileBytesIfExists(rootDir)
if err != nil {
return nil, err
}
if bytes != nil {
return &NodeDependencies{pjs, name, bytes}, nil

path, err = findValidLockfileInDir(rootDir)
if err == nil {
return &NodeDependencies{pjs, path}, nil
}
return nil, gcp.UserErrorf("lockfile not found")

return nil, gcp.UserErrorf("valid lock file not found", dir)
}

func readLockfileBytesIfExists(dir string) (string, []byte, error) {
func findValidLockfileInDir(dir string) (string, error) {
for _, filename := range possibleLockfileFilenames {
filePath := filepath.Join(dir, filename)
raw, err := os.ReadFile(filePath)
if os.IsNotExist(err) {
continue
}
if err != nil {
return "", nil, gcp.UserErrorf("Error while reading lockfile: %w", err)
if fp := filepath.Join(dir, filename); isValidLockFile(fp) {
return fp, nil
}
return filename, raw, nil
}
return "", nil, nil
return "", gcp.UserErrorf("valid lock file not found in directory %s", dir)
}

// isValidLockFile validates that the lock file both exists and is not empty.
func isValidLockFile(filePath string) bool {
info, err := os.Stat(filePath)
return err == nil && info.Size() > 0
}

// HasGCPBuild returns true if the given package.json file includes a "gcp-build" script.
Expand Down Expand Up @@ -358,7 +356,11 @@ func versionFromPnpmLock(rawPackageLock []byte, pkg string) (string, error) {
func versionFromYarnLock(rawPackageLock []byte, pjs *PackageJSON, pkg string) (string, error) {
// yarn requires custom parsing since it has a custom format
// this logic works for both yarn classic and berry
for _, dependency := range strings.Split(string(rawPackageLock[:]), "\n\n") {

// Split using a more flexible regex to handle various newline characters across OSes
dependencies := dependencyRegex.Split(string(rawPackageLock), -1)

for _, dependency := range dependencies {
if strings.Contains(dependency, pkg+"@") && strings.Contains(dependency, pjs.Dependencies[pkg]) {
for _, line := range strings.Split(dependency, "\n") {
if strings.Contains(line, "version") {
Expand All @@ -380,13 +382,17 @@ func versionFromNpmLock(rawPackageLock []byte, pkg string) (string, error) {

// Version tries to get the concrete package version used based on lock file.
func Version(deps *NodeDependencies, pkg string) (string, error) {
switch deps.LockfileName {
case "pnpm-lock.yaml":
return versionFromPnpmLock(deps.LockfileBytes, pkg)
case "yarn.lock":
return versionFromYarnLock(deps.LockfileBytes, deps.PackageJSON, pkg)
case "npm-shrinkwrap.json", "package-lock.json":
return versionFromNpmLock(deps.LockfileBytes, pkg)
raw, err := os.ReadFile(deps.LockfilePath)
if err != nil {
return "", gcp.UserErrorf("reading file at path %s: %w", deps.LockfilePath, err)
}
switch {
case strings.HasSuffix(deps.LockfilePath, "pnpm-lock.yaml"):
return versionFromPnpmLock(raw, pkg)
case strings.HasSuffix(deps.LockfilePath, "yarn.lock"):
return versionFromYarnLock(raw, deps.PackageJSON, pkg)
case strings.HasSuffix(deps.LockfilePath, "npm-shrinkwrap.json") || strings.HasSuffix(deps.LockfilePath, "package-lock.json"):
return versionFromNpmLock(raw, pkg)
}

return "", gcp.UserErrorf("Failed to find version for package %s", pkg)
Expand Down
158 changes: 36 additions & 122 deletions pkg/nodejs/nodejs_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,17 +345,7 @@ func TestReadNodeDependencies(t *testing.T) {
"c": "3.0",
},
},
LockfileName: "package-lock.json",
LockfileBytes: []byte(`{
"packages": {
"node_modules/a": {
"version": "1.0"
},
"node_modules/b": {
"version": "2.0"
}
}
}`),
LockfilePath: testdata.MustGetPath("testdata/test-read-node-deps/package-lock.json"),
}

testCases := []struct {
Expand All @@ -379,10 +369,13 @@ func TestReadNodeDependencies(t *testing.T) {
want: want,
},
{
name: "lockfile in root dir and package json in app dir",
name: "lockfile in root dir and package json in app dir, find lockfile in root dir",
rootDir: testdata.MustGetPath("testdata/test-read-node-deps-nested/"),
appDir: testdata.MustGetPath("testdata/test-read-node-deps-nested/package-a/"),
want: want,
want: &NodeDependencies{
PackageJSON: want.PackageJSON,
LockfilePath: testdata.MustGetPath("testdata/test-read-node-deps-nested/package-lock.json"),
},
},
}
for _, tc := range testCases {
Expand All @@ -401,175 +394,96 @@ func TestReadNodeDependencies(t *testing.T) {

func TestVersion(t *testing.T) {
testCases := []struct {
name string
nodeDeps *NodeDependencies
expectedVersion string
pkg string
expectedError bool
name string
nodeDeps *NodeDependencies
wantVersion string
pkg string
wantErr bool
}{
{
name: "Parses package-lock version nextjs",
name: "Parses package-lock.json version nextjs",
pkg: "next",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"next": "^13.1.0",
},
},
LockfileName: "package-lock.json",
LockfileBytes: []byte(`{
"packages": {
"node_modules/next": {
"version": "13.5.6"
}
}
}`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/nextjs-package-lock.json"),
},
expectedVersion: "13.5.6",
wantVersion: "14.1.4",
},
{
name: "Parses package-lock version angular",
pkg: "angular",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"angular": "^17.1.0",
},
},
LockfileName: "package-lock.json",
LockfileBytes: []byte(`{
"packages": {
"node_modules/angular": {
"version": "17.1.2"
}
}
}`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/angular-package-lock.json"),
},
expectedVersion: "17.1.2",
wantVersion: "17.1.3",
},
{
name: "Parses yarn.lock berry version nextjs",
pkg: "next",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"next": "^13.1.0",
"next": "14.1.4",
},
},
LockfileName: "yarn.lock",
LockfileBytes: []byte(`
"next@npm:^13.1.0":
version: 13.5.6`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/berry-yarn.lock"),
},
expectedVersion: "13.5.6",
wantVersion: "14.1.4",
},
{
name: "Parses yarn.lock classic version nextjs",
pkg: "next",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"next": "^13.1.0",
"next": "14.1.4",
},
},
LockfileName: "yarn.lock",
LockfileBytes: []byte(`
next@^13.1.0:
version: "13.5.6"
`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/classic-yarn.lock"),
},
expectedVersion: "13.5.6",
wantVersion: "14.1.4",
},
{
name: "Parses pnpm-lock v6 version nextjs",
pkg: "next",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"next": "^13.1.0",
},
},
LockfileName: "pnpm-lock.yaml",
LockfileBytes: []byte(`
dependencies:
next:
version: 13.5.6(@babel/[email protected])
`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/nextjs-v6-pnpm-lock.yaml"),
},
expectedVersion: "13.5.6",
wantVersion: "14.1.4",
},
{
name: "Parses pnpm-lock v9 version nextjs",
pkg: "next",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"next": "^13.1.0",
},
},
LockfileName: "pnpm-lock.yaml",
LockfileBytes: []byte(`
importers:
.:
dependencies:
next:
version: 13.5.6(@babel/[email protected])
`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/nextjs-v9-pnpm-lock.yaml"),
},
expectedVersion: "13.5.6",
wantVersion: "14.1.4",
},
{
name: "Parses pnpm-lock v6 version angular builder",
name: "Parses pnpm-lock v6 version angular",
pkg: "@angular-devkit/build-angular",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"@angular-devkit/build-angular": "^17.3.5",
},
},
LockfileName: "pnpm-lock.yaml",
LockfileBytes: []byte(`
devDependencies:
'@angular-devkit/build-angular':
version: 17.3.5(@angular/[email protected])
`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/angular-v6-pnpm-lock.yaml"),
},
expectedVersion: "17.3.5",
wantVersion: "17.3.6",
},
{
name: "Parses pnpm-lock v9 version angular builder",
name: "Parses pnpm-lock v9 version angular",
pkg: "@angular-devkit/build-angular",
nodeDeps: &NodeDependencies{
PackageJSON: &PackageJSON{
Dependencies: map[string]string{
"@angular-devkit/build-angular": "^17.3.5",
},
},
LockfileName: "pnpm-lock.yaml",
LockfileBytes: []byte(`
importers:
.:
devDependencies:
'@angular-devkit/build-angular':
version: 17.3.5(@angular/[email protected])
`),
LockfilePath: testdata.MustGetPath("testdata/lock-files/angular-v9-pnpm-lock.yaml"),
},
expectedVersion: "17.3.5",
wantVersion: "17.3.6",
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
version, err := Version(tc.nodeDeps, tc.pkg)
if version != tc.expectedVersion {
t.Fatalf("Version(%v, %v) output: %s doesn't match expected output %s", tc.nodeDeps, tc.pkg, version, tc.expectedVersion)
if version != tc.wantVersion {
t.Fatalf("Version(%v, %v) output: %s doesn't match expected output %s", tc.nodeDeps, tc.pkg, version, tc.wantVersion)
}
if gotErr := (err != nil); gotErr != tc.expectedError {
t.Fatalf("Version(_, %v) returned error: %v, wanted: %v, errMsg: %v.", tc.pkg, gotErr, tc.expectedError, err)
if gotErr := (err != nil); gotErr != tc.wantErr {
t.Fatalf("Version(_, %v) returned error: %v, wanted: %v, errMsg: %v.", tc.pkg, gotErr, tc.wantErr, err)
}
})
}
Expand Down
Loading

0 comments on commit f335a79

Please sign in to comment.