Skip to content

Commit

Permalink
Change Symlink()/Readlink() to take/return path.Parser
Browse files Browse the repository at this point in the history 
On Windows, symlink targets may contain drive letters and are supposed
to use backslashes. This is problematic, because the Directory.Symlink()
and Directory.Readlink() methods are currently supposed to return
UNIX-like pathname strings. By altering these functions to use
path.Parser, users of this API don't need to care about pathname
conventions used on the host operating system.
  • Loading branch information
@EdSchouten
EdSchouten committed Mar 8, 2024
1 parent 19e35d0 commit 0dd19de
Show file tree
Hide file tree
Showing 5 changed files with 16 additions and 16 deletions.
4 changes: 2 additions & 2 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ replace github.com/grpc-ecosystem/grpc-gateway/v2 => github.com/grpc-ecosystem/g

require (
github.com/bazelbuild/remote-apis v0.0.0-20240215191509-9ff14cecffe5
github.com/buildbarn/bb-remote-execution v0.0.0-20240307211444-6879a90a1d00
github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5
github.com/buildbarn/bb-remote-execution v0.0.0-20240308103551-b4cdefde2e35
github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef
golang.org/x/sync v0.6.0
google.golang.org/genproto/googleapis/bytestream v0.0.0-20240228224816-df926f6c8641
google.golang.org/grpc v1.62.0
Expand Down
4 changes: 4 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,14 @@ github.com/buildbarn/bb-remote-execution v0.0.0-20240306174844-94b3776d5dba h1:p
github.com/buildbarn/bb-remote-execution v0.0.0-20240306174844-94b3776d5dba/go.mod h1:qwNvc1PxPWzRzcKiQ/Hq1MQmxB8fjWQ+Lv0h9r8aysU=
github.com/buildbarn/bb-remote-execution v0.0.0-20240307211444-6879a90a1d00 h1:Pz2SMTPFEbrxvfpGt2f0VMj6ZooIW5GMDH0MaIk6tIU=
github.com/buildbarn/bb-remote-execution v0.0.0-20240307211444-6879a90a1d00/go.mod h1:R/PblhgyLNtqYySF5xvpjbMzfmGYW1HWrPvwN6q0FQU=
github.com/buildbarn/bb-remote-execution v0.0.0-20240308103551-b4cdefde2e35 h1:bH+vSGzh/bBgV8KC4oAhlFmjoUT2TIZ8RO9NlfTgZYc=
github.com/buildbarn/bb-remote-execution v0.0.0-20240308103551-b4cdefde2e35/go.mod h1:eWUQ51dc8XX2eKM4re9wD8VBzJ8D21JZi87L7mfttus=
github.com/buildbarn/bb-storage v0.0.0-20240227100204-0aa40dfdbead h1:fHapKnQQLgJaMxGiBAUCPVHNfD5vV1LDfXqmyClJ6Lc=
github.com/buildbarn/bb-storage v0.0.0-20240227100204-0aa40dfdbead/go.mod h1:gHT0PInDFOV/JZjeeNwvqmn33MKHHyk3V18e4/Cs/jM=
github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5 h1:tBbub4L03HT0PNKToki245kaXjwb+AOzyhfy8oosL6w=
github.com/buildbarn/bb-storage v0.0.0-20240307194821-4d0d7a3d85b5/go.mod h1:0uISGKJD6Owt29w2sUlK0TeLtYdLWtBiC43yVHdgMAY=
github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef h1:4Mq41hEzT6G4F1dxNiwbwHsQf/pNSFk//pkVgZbGbcw=
github.com/buildbarn/bb-storage v0.0.0-20240308085957-e8fd6935d2ef/go.mod h1:0uISGKJD6Owt29w2sUlK0TeLtYdLWtBiC43yVHdgMAY=
github.com/buildbarn/go-xdr v0.0.0-20231115101217-a9e2aa4cf64b h1:/sKWC0Fs5fXNo/t72BRZRLERg4v2gFoEeg2Mk+a8xak=
github.com/buildbarn/go-xdr v0.0.0-20231115101217-a9e2aa4cf64b/go.mod h1:VwInghBSUyPtNBhl7o2oCUnxOCTGgySJnRTO1Kh7XuI=
github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
Expand Down
8 changes: 4 additions & 4 deletions go_dependencies.bzl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ def go_dependencies():
go_repository(
name = "com_github_buildbarn_bb_remote_execution",
importpath = "github.com/buildbarn/bb-remote-execution",
sum = "h1:Pz2SMTPFEbrxvfpGt2f0VMj6ZooIW5GMDH0MaIk6tIU=",
version = "v0.0.0-20240307211444-6879a90a1d00",
sum = "h1:bH+vSGzh/bBgV8KC4oAhlFmjoUT2TIZ8RO9NlfTgZYc=",
version = "v0.0.0-20240308103551-b4cdefde2e35",
)
go_repository(
name = "com_github_buildbarn_bb_storage",
importpath = "github.com/buildbarn/bb-storage",
sum = "h1:tBbub4L03HT0PNKToki245kaXjwb+AOzyhfy8oosL6w=",
version = "v0.0.0-20240307194821-4d0d7a3d85b5",
sum = "h1:4Mq41hEzT6G4F1dxNiwbwHsQf/pNSFk//pkVgZbGbcw=",
version = "v0.0.0-20240308085957-e8fd6935d2ef",
)
go_repository(
name = "com_github_buildbarn_go_xdr",
Expand Down
6 changes: 1 addition & 5 deletions pkg/filesystem/virtual/bazel_output_service_directory.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -558,17 +558,13 @@ func (cw *statWalker) OnDirectory(name path.Component) (path.GotDirectoryOrSymli
} else if err != nil {
return nil, err
}
targetParser, err := path.NewUNIXParser(target)
if err != nil {
return nil, err
}

// Got a symbolic link in the middle of a path. Those should
// always be followed.
cw.stat = &bazeloutputservice.BatchStatResponse_Stat{}
return path.GotSymlink{
Parent: cw,
Target: targetParser,
Target: target,
}, nil
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/filesystem/virtual/bazel_output_service_directory_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,7 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
t.Run("OnDirectoryReadlinkFailure", func(t *testing.T) {
leaf := mock.NewMockNativeLeaf(ctrl)
outputPath.EXPECT().LookupChild(path.MustNewComponent("stdio")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf), nil)
leaf.EXPECT().Readlink().Return("", status.Error(codes.Internal, "Disk failure"))
leaf.EXPECT().Readlink().Return(nil, status.Error(codes.Internal, "Disk failure"))

_, err := d.BatchStat(ctx, &bazeloutputservice.BatchStatRequest{
BuildId: "37f5dbef-b117-4fb6-bce8-5c147cb603b4",
Expand Down Expand Up @@ -832,7 +832,7 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory2), nil)
leaf2 := mock.NewMockNativeLeaf(ctrl)
directory2.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_relative_directory")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf2), nil)
leaf2.EXPECT().Readlink().Return("..", nil)
leaf2.EXPECT().Readlink().Return(path.MustNewUNIXParser(".."), nil)

// Lookup of "nested/symlink_internal_absolute_path",
// being a symlink containing an absolute path starting
Expand All @@ -841,7 +841,7 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory3), nil)
leaf3 := mock.NewMockNativeLeaf(ctrl)
directory3.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_absolute_path")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf3), nil)
leaf3.EXPECT().Readlink().Return("/home/bob/bb_clientd/outputs/9da951b8cb759233037166e28f7ea186/hello", nil)
leaf3.EXPECT().Readlink().Return(path.MustNewUNIXParser("/home/bob/bb_clientd/outputs/9da951b8cb759233037166e28f7ea186/hello"), nil)
directory4 := mock.NewMockPrepopulatedDirectory(ctrl)
outputPath.EXPECT().LookupChild(path.MustNewComponent("hello")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory4), nil)

Expand All @@ -852,7 +852,7 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory5), nil)
leaf4 := mock.NewMockNativeLeaf(ctrl)
directory5.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_absolute_alias")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf4), nil)
leaf4.EXPECT().Readlink().Return("/home/bob/.cache/bazel/_bazel_bob/9da951b8cb759233037166e28f7ea186/execroot/myproject/bazel-out/hello", nil)
leaf4.EXPECT().Readlink().Return(path.MustNewUNIXParser("/home/bob/.cache/bazel/_bazel_bob/9da951b8cb759233037166e28f7ea186/execroot/myproject/bazel-out/hello"), nil)
directory6 := mock.NewMockPrepopulatedDirectory(ctrl)
outputPath.EXPECT().LookupChild(path.MustNewComponent("hello")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory6), nil)

Expand All @@ -863,7 +863,7 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory7), nil)
leaf5 := mock.NewMockNativeLeaf(ctrl)
directory7.EXPECT().LookupChild(path.MustNewComponent("symlink_external")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf5), nil)
leaf5.EXPECT().Readlink().Return("/etc/passwd", nil)
leaf5.EXPECT().Readlink().Return(path.MustNewUNIXParser("/etc/passwd"), nil)

// Lookup of "nonexistent".
outputPath.EXPECT().LookupChild(path.MustNewComponent("nonexistent")).Return(re_vfs.PrepopulatedDirectoryChild{}, syscall.ENOENT)
Expand Down

0 comments on commit 0dd19de

Please sign in to comment.