From ef239a11844d72e644e2e62fc3dfc60ec77dbc30 Mon Sep 17 00:00:00 2001 From: Ye Kuang Date: Tue, 14 Jul 2020 23:15:54 +0900 Subject: [PATCH] Add symlink info to file metadata (#153) * Add symlink info to file metadata * use pointer type * simplify * remove print * IsDangling * fix tests * fmt * rm unused * fix msg * set target even if dangling * apply fix * func name * rm testFileParams --- go.sum | 2 + go/pkg/filemetadata/filemetadata.go | 34 ++++++-- go/pkg/filemetadata/filemetadata_test.go | 104 ++++++++++++++++++++++- go/pkg/tree/tree.go | 6 +- 4 files changed, 135 insertions(+), 11 deletions(-) diff --git a/go.sum b/go.sum index ff67409e6..d51ed3c15 100644 --- a/go.sum +++ b/go.sum @@ -32,6 +32,7 @@ github.com/bazelbuild/remote-apis v0.0.0-20190507145712-5556e9c6153f h1:KLZodm2m github.com/bazelbuild/remote-apis v0.0.0-20190507145712-5556e9c6153f/go.mod h1:9Y+1FnaNUGVV6wKE0Jdh+mguqDUsyd9uUqokalrC7DQ= github.com/bazelbuild/remote-apis v0.0.0-20191104140458-e77c4eb2ca48 h1:bgj+Oufa8F4rCHe/8omhml7cBlg3VmNhF66ed1vT2Bw= github.com/bazelbuild/remote-apis v0.0.0-20191104140458-e77c4eb2ca48/go.mod h1:9Y+1FnaNUGVV6wKE0Jdh+mguqDUsyd9uUqokalrC7DQ= +github.com/bazelbuild/remote-apis v0.0.0-20200624085034-0afc3700d177 h1:JeDdP1ZsFOMctOzhcwYfLkGaLT8Nzxc1AXkXKUrWbAw= github.com/boombuler/barcode v1.0.0 h1:s1TvRnXwL2xJRaccrdcBQMZxq6X7DvsMogtmJeHDdrc= github.com/boombuler/barcode v1.0.0/go.mod h1:paBWMcWSl3LHKBqUq+rly7CNSldXjb2rDl3JlRe0mD8= github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625 h1:ckJgFhFWywOx+YLEMIJsTb+NV6NexWICk5+AMSuz3ss= @@ -111,6 +112,7 @@ github.com/golang/protobuf v1.3.2 h1:6nsPYzhq5kReh6QImI3k5qWzO4PEbvbIW2cwSfR/6xs github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.3 h1:gyjaxf+svBWX08ZjK86iN9geUJF0H6gp2IRKX6Nf6/I= github.com/golang/protobuf v1.3.3/go.mod h1:vzj43D7+SQXF/4pzW/hwtAqwc6iTitCiVSaWz5lYuqw= +github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db h1:woRePGFeVFfLKN/pOkfl+p/TAqKOfFu+7KPlMVpok/w= github.com/golang/snappy v0.0.0-20180518054509-2e65f85255db/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/gonum/blas v0.0.0-20181208220705-f22b278b28ac h1:Q0Jsdxl5jbxouNs1TQYt0gxesYMU4VXRbsTlgDloZ50= diff --git a/go/pkg/filemetadata/filemetadata.go b/go/pkg/filemetadata/filemetadata.go index ccec09835..e3bb5c83c 100644 --- a/go/pkg/filemetadata/filemetadata.go +++ b/go/pkg/filemetadata/filemetadata.go @@ -8,19 +8,25 @@ import ( "github.com/bazelbuild/remote-apis-sdks/go/pkg/digest" ) +// SymlinkMetadata contains details if the given path is a symlink. +type SymlinkMetadata struct { + Target string + IsDangling bool +} + // Metadata contains details for a particular file. type Metadata struct { Digest digest.Digest IsExecutable bool Err error + Symlink *SymlinkMetadata } // FileError is the error returned by the Compute function. type FileError struct { - IsDirectory bool - IsNotFound bool - IsInvalidSymlink bool - Err error + IsDirectory bool + IsNotFound bool + Err error } // Error returns the error message. @@ -41,11 +47,25 @@ func isSymlink(filename string) (bool, error) { func Compute(filename string) *Metadata { md := &Metadata{Digest: digest.Empty} file, err := os.Stat(filename) + if isSym, _ := isSymlink(filename); isSym { + md.Symlink = &SymlinkMetadata{} + if dest, rlErr := os.Readlink(filename); rlErr != nil { + md.Err = &FileError{Err: rlErr} + return md + } else { + // If Readlink was OK, we set Target, even if this could be a dangling symlink. + md.Symlink.Target = dest + } + + if err != nil { + md.Err = &FileError{Err: err} + md.Symlink.IsDangling = true + return md + } + } + if err != nil { fe := &FileError{Err: err} - if s, err := isSymlink(filename); err == nil && s { - fe.IsInvalidSymlink = true - } if os.IsNotExist(err) { fe.IsNotFound = true } diff --git a/go/pkg/filemetadata/filemetadata_test.go b/go/pkg/filemetadata/filemetadata_test.go index bc895830e..b7ffc70ee 100644 --- a/go/pkg/filemetadata/filemetadata_test.go +++ b/go/pkg/filemetadata/filemetadata_test.go @@ -3,13 +3,14 @@ package filemetadata import ( "io/ioutil" "os" + "path/filepath" "testing" "github.com/bazelbuild/remote-apis-sdks/go/pkg/digest" "github.com/google/go-cmp/cmp" ) -func TestCompute(t *testing.T) { +func TestComputeFiles(t *testing.T) { tests := []struct { name string contents string @@ -66,6 +67,90 @@ func TestComputeDirectory(t *testing.T) { } } +func TestComputeSymlinksToFile(t *testing.T) { + tests := []struct { + name string + contents string + executable bool + }{ + { + name: "valid", + contents: "bla", + }, + { + name: "valid-executable", + contents: "executable", + executable: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + symlinkPath := filepath.Join(os.TempDir(), tc.name) + defer os.RemoveAll(symlinkPath) + targetPath, err := createSymlinkToFile(t, symlinkPath, tc.executable, tc.contents) + if err != nil { + t.Fatalf("Failed to create tmp symlink for testing digests: %v", err) + } + got := Compute(symlinkPath) + + if got.Err != nil { + t.Errorf("Compute(%v) failed. Got error: %v", symlinkPath, got.Err) + } + want := &Metadata{ + Symlink: &SymlinkMetadata{ + Target: targetPath, + IsDangling: false, + }, + Digest: digest.NewFromBlob([]byte(tc.contents)), + IsExecutable: tc.executable, + } + + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Compute(%v) returned diff. (-want +got)\n%s", symlinkPath, diff) + } + }) + } +} + +func TestComputeDanglingSymlinks(t *testing.T) { + // Create a temporary fake target so that os.Symlink() can work. + symlinkPath := filepath.Join(os.TempDir(), "dangling") + defer os.RemoveAll(symlinkPath) + targetPath, err := createSymlinkToFile(t, symlinkPath, false, "transient") + if err != nil { + t.Fatalf("Failed to create tmp symlink for testing digests: %v", err) + } + // Make the symlink dangling + os.RemoveAll(targetPath) + + got := Compute(symlinkPath) + if got.Err == nil || !got.Symlink.IsDangling { + t.Errorf("Compute(%v) should fail because the symlink is dangling", symlinkPath) + } + if got.Symlink.Target != targetPath { + t.Errorf("Compute(%v) should still set Target for the dangling symlink, want=%v got=%v", symlinkPath, targetPath, got.Symlink.Target) + } +} + +func TestComputeSymlinksToDirectory(t *testing.T) { + symlinkPath := filepath.Join(os.TempDir(), "dir-symlink") + defer os.RemoveAll(symlinkPath) + targetPath, err := ioutil.TempDir(os.TempDir(), "") + if err != nil { + t.Fatalf("Failed to create tmp directory: %v", err) + } + err = createSymlinkToTarget(t, symlinkPath, targetPath) + + if err != nil { + t.Fatalf("Failed to create tmp symlink for testing digests: %v", err) + } + + got := Compute(symlinkPath) + if fe, ok := got.Err.(*FileError); !ok || !fe.IsDirectory { + t.Errorf("Compute(%v).Err = %v, want FileError{IsDirectory:true}", symlinkPath, got.Err) + } +} + func createFile(t *testing.T, executable bool, contents string) (string, error) { t.Helper() perm := os.FileMode(0666) @@ -88,3 +173,20 @@ func createFile(t *testing.T, executable bool, contents string) (string, error) } return filename, nil } + +func createSymlinkToFile(t *testing.T, symlinkPath string, executable bool, contents string) (string, error) { + t.Helper() + targetPath, err := createFile(t, executable, contents) + if err != nil { + return "", err + } + if err := createSymlinkToTarget(t, symlinkPath, targetPath); err != nil { + return "", err + } + return targetPath, nil +} + +func createSymlinkToTarget(t *testing.T, symlinkPath string, targetPath string) error { + t.Helper() + return os.Symlink(targetPath, symlinkPath) +} diff --git a/go/pkg/tree/tree.go b/go/pkg/tree/tree.go index c3025f634..aae343bf5 100644 --- a/go/pkg/tree/tree.go +++ b/go/pkg/tree/tree.go @@ -65,14 +65,14 @@ func loadFiles(execRoot string, excl []*command.InputExclusion, path string, fs absPath := filepath.Clean(filepath.Join(execRoot, path)) meta := cache.Get(absPath) t := command.FileInputType + if smd := meta.Symlink; smd != nil && smd.IsDangling { + return nil + } if meta.Err != nil { e, ok := meta.Err.(*filemetadata.FileError) if !ok { return meta.Err } - if e.IsInvalidSymlink { - return nil - } if !e.IsDirectory { return meta.Err }