Skip to content

Commit

Permalink
Use dir list to get file mode for directory entries
Browse files Browse the repository at this point in the history
  • Loading branch information
Tatskaari committed Nov 13, 2023
1 parent d66acd8 commit 058e913
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 14 deletions.
27 changes: 15 additions & 12 deletions src/remote/fs/fs.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ func (fs *fs) open(path, name string, wd *pb.Directory) (iofs.File, error) {
name: name,
isDir: true,
},
pb: dirPb,
pb: dirPb,
children: fs.directories,
}, nil
}
return fs.open(filepath.Join(path, name), rest, dirPb)
Expand Down Expand Up @@ -102,8 +103,7 @@ func (fs *fs) open(path, name string, wd *pb.Directory) (iofs.File, error) {
for _, l := range wd.Symlinks {
if l.Name == name {
if filepath.IsAbs(l.Target) {
// The doc comments suggest that sometimes this is supported. I'm not sure how this would be useful in
// the context of Please so I'll just return an error here to assert it's never used.
// Some REAPI implementations support this, but this is considered invalid where Please is concerned.
path = filepath.Join(path, name)
return nil, fmt.Errorf("symlink %v is has abs target %v. This is not supported", path, l.Target)
}
Expand Down Expand Up @@ -144,28 +144,30 @@ func (b *file) Close() error {
}

type dir struct {
pb *pb.Directory
pb *pb.Directory
children map[digest.Digest]*pb.Directory
info
}

// ReadDir is a slightly incorrect implementation of ReadDir. It doesn't return all the information you would normally
// have with a filesystem. We can't know this information without downloading more digests from the client, however we
// likely will never need this. This is enough to facilitate globbing.
// ReadDir is a slightly incorrect implementation of ReadDir. It deviates slightly as it will report all files have 0
// size. This seems to work for our limited purposes though.
func (p *dir) ReadDir(n int) ([]iofs.DirEntry, error) {
dirSize := n
if n <= 0 {
dirSize = len(p.pb.Files) + len(p.pb.Symlinks) + len(p.pb.Files)
}
ret := make([]iofs.DirEntry, 0, dirSize)
for _, dir := range p.pb.Directories {
for _, dirNode := range p.pb.Directories {
if n > 0 && len(ret) == n {
return ret, nil
}
dir := p.children[digest.NewFromProtoUnvalidated(dirNode.Digest)]
ret = append(ret, &info{
name: dir.Name,
name: dirNode.Name,
isDir: true,
typeMode: os.ModeDir,
mode: 0, // We can't know this without downloading the Directory proto
mode: os.FileMode(dir.NodeProperties.UnixMode.Value),
modTime: dir.NodeProperties.GetMtime().AsTime(),
})
}
for _, file := range p.pb.Files {
Expand All @@ -175,7 +177,9 @@ func (p *dir) ReadDir(n int) ([]iofs.DirEntry, error) {
ret = append(ret, &info{
name: file.Name,
mode: os.FileMode(file.NodeProperties.UnixMode.Value),
size: 0, // We can't know this without downloading the file.
// TODO(jpoole): technically we could calculate this on demand by allowing info.Size() to download the file
// from the CAS... we don't need to for now though.
size: 0,
})
}
for _, link := range p.pb.Symlinks {
Expand All @@ -186,7 +190,6 @@ func (p *dir) ReadDir(n int) ([]iofs.DirEntry, error) {
name: link.Name,
mode: os.FileMode(link.NodeProperties.UnixMode.Value),
typeMode: os.ModeSymlink,
size: 0, // We can't know this without downloading the file.
})
}
return ret, nil
Expand Down
24 changes: 23 additions & 1 deletion src/remote/fs/fs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestFS(t *testing.T) {
// . (root)
// |- foo (file containing wibble wibble wibble)
// |- bar
// |- empty (an empty directory)
// |- foo (same file as above)
// |- example.go
// |- example_test.go
Expand All @@ -48,6 +49,13 @@ func TestFS(t *testing.T) {
Digest: fooDigest.ToProto(),
}

empty := &pb.Directory{
NodeProperties: &pb.NodeProperties{UnixMode: &wrappers.UInt32Value{
Value: 0777,
}}}
emptyDigest, err := digest.NewFromMessage(empty)
require.NoError(t, err)

bar := &pb.Directory{
Files: []*pb.FileNode{
foo,
Expand Down Expand Up @@ -82,6 +90,12 @@ func TestFS(t *testing.T) {
}},
},
},
Directories: []*pb.DirectoryNode{
{
Name: "empty",
Digest: emptyDigest.ToProto(),
},
},
NodeProperties: &pb.NodeProperties{UnixMode: &wrappers.UInt32Value{
Value: 0777,
}},
Expand Down Expand Up @@ -111,6 +125,7 @@ func TestFS(t *testing.T) {
Root: root,
Children: []*pb.Directory{
bar,
empty,
},
}

Expand All @@ -133,7 +148,14 @@ func TestFS(t *testing.T) {

entries, err := iofs.ReadDir(fs, "bar")
require.NoError(t, err)
assert.Len(t, entries, 5)
assert.Len(t, entries, 6)

for _, e := range entries {
i, err := e.Info()
require.NoError(t, err)
// We set them all to 0777 above
assert.Equal(t, iofs.FileMode(0777), i.Mode(), "%v mode was wrong", e.Name())
}

matches, err := iofs.Glob(fs, "bar/*.go")
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion src/remote/fs/info.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func (i *info) ModTime() time.Time {
}

func (i *info) IsDir() bool {
return false
return i.isDir
}

func (i *info) Sys() any {
Expand Down

0 comments on commit 058e913

Please sign in to comment.