Let symlinks be backed by path.Parser

MODULE.bazel

@@ -24,7 +24,7 @@ git_override(
 
 git_override(
     module_name = "com_github_buildbarn_bb_remote_execution",
-    commit = "9b1e3637cd58e1d59992f97012ab47d9d9af6cd6",
+    commit = "cd89f0554d18a1e8a0f3f4e3155b273fb2d3fff7",
     remote = "https://github.com/buildbarn/bb-remote-execution.git",
 )
 

cmd/bb_clientd/main.go

@@ -218,7 +218,9 @@ func main() {
 		// allowing data to be loaded lazily.
 		symlinkFactory := re_vfs.NewHandleAllocatingSymlinkFactory(
 			re_vfs.BaseSymlinkFactory,
-			rootHandleAllocator.New())
+			rootHandleAllocator.New(),
+			path.LocalFormat,
+		)
 		outputPathFactory := cd_vfs.NewInMemoryOutputPathFactory(filePool, symlinkFactory, rootHandleAllocator, sort.Sort, clock.SystemClock)
 		if persistencyConfiguration := configuration.OutputPathPersistency; persistencyConfiguration != nil {
 			// Upload local files at the end of every build. This

go.mod

@@ -14,7 +14,7 @@ replace github.com/envoyproxy/protoc-gen-validate => github.com/envoyproxy/proto
 require (
 	github.com/bazelbuild/buildtools v0.0.0-20260317083046-eb4b727fa099
 	github.com/bazelbuild/remote-apis v0.0.0-20260216160025-715b73f3f9e4
-	github.com/buildbarn/bb-remote-execution v0.0.0-20260303143927-9b1e3637cd58
+	github.com/buildbarn/bb-remote-execution v0.0.0-20260319042723-cd89f0554d18
 	github.com/buildbarn/bb-storage v0.0.0-20260317135248-dc342e1799d7
 	github.com/stretchr/testify v1.11.1
 	go.uber.org/mock v0.6.0

go.sum

@@ -117,8 +117,8 @@ github.com/bazelbuild/rules_go v0.59.0/go.mod h1:Pn30cb4M513fe2rQ6GiJ3q8QyrRsgC7
 github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
 github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
-github.com/buildbarn/bb-remote-execution v0.0.0-20260303143927-9b1e3637cd58 h1:/Hpv5YjmbuF+05pSESFdGtMHW59AIiXSF/Fz263a47Q=
-github.com/buildbarn/bb-remote-execution v0.0.0-20260303143927-9b1e3637cd58/go.mod h1:SNij+nQt7w7n51VmQ6pgbp/CM7gdclAHbIyi4Mn4KPM=
+github.com/buildbarn/bb-remote-execution v0.0.0-20260319042723-cd89f0554d18 h1:xDjczRdWxG2EIhqZbIR8gYLTKKLZezTFxKKe0fkihCo=
+github.com/buildbarn/bb-remote-execution v0.0.0-20260319042723-cd89f0554d18/go.mod h1:CQ78VViU6TTHLoxfe1xh8XOm8fJNMJuwOlWxiEEWB58=
 github.com/buildbarn/bb-storage v0.0.0-20260317135248-dc342e1799d7 h1:n6sJ9PxC6WSaLboSi3EcbIxfqgRd7tulb5hiep4kFh4=
 github.com/buildbarn/bb-storage v0.0.0-20260317135248-dc342e1799d7/go.mod h1:96kqnkrdkHHi94Agje3NM8qwrYMxJRSkAqsb7oXRhNI=
 github.com/buildbarn/go-sha256tree v0.0.0-20250310211320-0f70f20e855b h1:IKUxixGBm9UxobU7c248z0BF0ojG19uoSLz8MFZM/KA=

pkg/filesystem/virtual/bazel_output_service_directory.go

@@ -512,6 +512,7 @@ func (d *BazelOutputServiceDirectory) StageArtifacts(ctx context.Context, reques
 // corresponding to a requested path. It is capable of expanding
 // symbolic links, if encountered.
 type statWalker struct {
+	context        context.Context
 	digestFunction *digest.Function
 
 	stack util.NonEmptyStack[virtual.PrepopulatedDirectory]
@@ -551,22 +552,19 @@ func (cw *statWalker) OnDirectory(name path.Component) (path.GotDirectoryOrSymli
 		}, nil
 	}
 
-	var p virtual.ApplyReadlink
-	if !leaf.VirtualApply(&p) {
-		panic("output path contains leaves that don't support ApplyReadlink")
-	}
-	if p.Err == syscall.EINVAL {
+	var attributes virtual.Attributes
+	leaf.VirtualGetAttributes(cw.context, virtual.AttributesMaskSymlinkTarget, &attributes)
+	target, ok := attributes.GetSymlinkTarget()
+	if !ok {
 		return nil, syscall.ENOTDIR
-	} else if p.Err != nil {
-		return nil, p.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: p.Target,
+		Target: target,
 	}, nil
 }
 
@@ -627,6 +625,7 @@ func (d *BazelOutputServiceDirectory) BatchStat(ctx context.Context, request *ba
 	}
 	for _, statPath := range request.Paths {
 		statWalker := statWalker{
+			context:        ctx,
 			digestFunction: &buildState.digestFunction,
 			stack:          util.NewNonEmptyStack[virtual.PrepopulatedDirectory](outputPathState.rootDirectory),
 			stat:           &bazeloutputservice.BatchStatResponse_Stat{},

pkg/filesystem/virtual/bazel_output_service_directory_test.go

@@ -821,23 +821,6 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
 		testutil.RequireEqualStatus(t, status.Error(codes.Internal, "Failed to resolve path \"stdio/printf.o\" beyond \".\": Disk failure"), err)
 	})
 
-	t.Run("OnDirectoryReadlinkFailure", func(t *testing.T) {
-		leaf := mock.NewMockLinkableLeaf(ctrl)
-		outputPath.EXPECT().LookupChild(path.MustNewComponent("stdio")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf), nil)
-		leaf.EXPECT().VirtualApply(gomock.Any()).
-			Do(func(data any) {
-				p := data.(*re_vfs.ApplyReadlink)
-				p.Err = status.Error(codes.Internal, "Disk failure")
-			}).
-			Return(true)
-
-		_, err := d.BatchStat(ctx, &bazeloutputservice.BatchStatRequest{
-			BuildId: "37f5dbef-b117-4fb6-bce8-5c147cb603b4",
-			Paths:   []string{"stdio/printf.o"},
-		})
-		testutil.RequireEqualStatus(t, status.Error(codes.Internal, "Failed to resolve path \"stdio/printf.o\" beyond \".\": Disk failure"), err)
-	})
-
 	t.Run("OnTerminalLookupFailure", func(t *testing.T) {
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("printf.o")).Return(re_vfs.PrepopulatedDirectoryChild{}, status.Error(codes.Internal, "Disk failure"))
 
@@ -899,12 +882,10 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory2), nil)
 		leaf2 := mock.NewMockLinkableLeaf(ctrl)
 		directory2.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_relative_directory")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf2), nil)
-		leaf2.EXPECT().VirtualApply(gomock.Any()).
-			Do(func(data any) {
-				p := data.(*re_vfs.ApplyReadlink)
-				p.Target = path.UNIXFormat.NewParser("..")
-			}).
-			Return(true)
+		leaf2.EXPECT().VirtualGetAttributes(gomock.Any(), re_vfs.AttributesMaskSymlinkTarget, gomock.Any()).
+			Do(func(ctx context.Context, requested re_vfs.AttributesMask, attributes *re_vfs.Attributes) {
+				attributes.SetSymlinkTarget(path.UNIXFormat.NewParser(".."))
+			})
 
 		// Lookup of "nested/symlink_internal_absolute_path",
 		// being a symlink containing an absolute path starting
@@ -913,12 +894,10 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory3), nil)
 		leaf3 := mock.NewMockLinkableLeaf(ctrl)
 		directory3.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_absolute_path")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf3), nil)
-		leaf3.EXPECT().VirtualApply(gomock.Any()).
-			Do(func(data any) {
-				p := data.(*re_vfs.ApplyReadlink)
-				p.Target = path.UNIXFormat.NewParser("/home/bob/bb_clientd/outputs/9da951b8cb759233037166e28f7ea186/hello")
-			}).
-			Return(true)
+		leaf3.EXPECT().VirtualGetAttributes(gomock.Any(), re_vfs.AttributesMaskSymlinkTarget, gomock.Any()).
+			Do(func(ctx context.Context, requested re_vfs.AttributesMask, attributes *re_vfs.Attributes) {
+				attributes.SetSymlinkTarget(path.UNIXFormat.NewParser("/home/bob/bb_clientd/outputs/9da951b8cb759233037166e28f7ea186/hello"))
+			})
 		directory4 := mock.NewMockPrepopulatedDirectory(ctrl)
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("hello")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory4), nil)
 
@@ -929,12 +908,10 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory5), nil)
 		leaf4 := mock.NewMockLinkableLeaf(ctrl)
 		directory5.EXPECT().LookupChild(path.MustNewComponent("symlink_internal_absolute_alias")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf4), nil)
-		leaf4.EXPECT().VirtualApply(gomock.Any()).
-			Do(func(data any) {
-				p := data.(*re_vfs.ApplyReadlink)
-				p.Target = path.UNIXFormat.NewParser("/home/bob/.cache/bazel/_bazel_bob/9da951b8cb759233037166e28f7ea186/execroot/myproject/bazel-out/hello")
-			}).
-			Return(true)
+		leaf4.EXPECT().VirtualGetAttributes(gomock.Any(), re_vfs.AttributesMaskSymlinkTarget, gomock.Any()).
+			Do(func(ctx context.Context, requested re_vfs.AttributesMask, attributes *re_vfs.Attributes) {
+				attributes.SetSymlinkTarget(path.UNIXFormat.NewParser("/home/bob/.cache/bazel/_bazel_bob/9da951b8cb759233037166e28f7ea186/execroot/myproject/bazel-out/hello"))
+			})
 		directory6 := mock.NewMockPrepopulatedDirectory(ctrl)
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("hello")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory6), nil)
 
@@ -945,12 +922,10 @@ func TestBazelOutputServiceDirectoryBatchStat(t *testing.T) {
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("nested")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromDirectory(directory7), nil)
 		leaf5 := mock.NewMockLinkableLeaf(ctrl)
 		directory7.EXPECT().LookupChild(path.MustNewComponent("symlink_external")).Return(re_vfs.PrepopulatedDirectoryChild{}.FromLeaf(leaf5), nil)
-		leaf5.EXPECT().VirtualApply(gomock.Any()).
-			Do(func(data any) {
-				p := data.(*re_vfs.ApplyReadlink)
-				p.Target = path.UNIXFormat.NewParser("/etc/passwd")
-			}).
-			Return(true)
+		leaf5.EXPECT().VirtualGetAttributes(gomock.Any(), re_vfs.AttributesMaskSymlinkTarget, gomock.Any()).
+			Do(func(ctx context.Context, requested re_vfs.AttributesMask, attributes *re_vfs.Attributes) {
+				attributes.SetSymlinkTarget(path.UNIXFormat.NewParser("/etc/passwd"))
+			})
 
 		// Lookup of "nonexistent".
 		outputPath.EXPECT().LookupChild(path.MustNewComponent("nonexistent")).Return(re_vfs.PrepopulatedDirectoryChild{}, syscall.ENOENT)

pkg/filesystem/virtual/blob_access_command_file_factory.go

@@ -134,10 +134,6 @@ func (f *commandFile) VirtualRead(buf []byte, offset uint64) (int, bool, virtual
 	return len(buf), eof, virtual.StatusOK
 }
 
-func (f *commandFile) VirtualReadlink(ctx context.Context) ([]byte, virtual.Status) {
-	return nil, virtual.StatusErrInval
-}
-
 func (f *commandFile) VirtualClose(shareAccess virtual.ShareMask) {}
 
 func (f *commandFile) VirtualWrite(buf []byte, offset uint64) (int, virtual.Status) {

pkg/filesystem/virtual/cas_directory.go

@@ -58,11 +58,17 @@ func (d *casDirectory) createSelf() virtual.Directory {
 	return d.handleAllocator.New(bytes.NewBuffer([]byte{0})).AsStatelessDirectory(d)
 }
 
-func (d *casDirectory) createSymlink(index uint64, target string) virtual.LinkableLeaf {
+func (d *casDirectory) createSymlink(index uint64, target string) (virtual.LinkableLeaf, virtual.Status) {
+	symlink, err := virtual.BaseSymlinkFactory.LookupSymlink(path.UNIXFormat.NewParser(target))
+	if err != nil {
+		d.directoryContext.LogError(util.StatusWrapf(err, "Failed to create symbolic link with target %#v", target))
+		return nil, virtual.StatusErrIO
+	}
+
 	var encodedIndex [binary.MaxVarintLen64]byte
 	return d.handleAllocator.
 		New(bytes.NewBuffer(encodedIndex[:binary.PutUvarint(encodedIndex[:], index+1)])).
-		AsLinkableLeaf(virtual.BaseSymlinkFactory.LookupSymlink([]byte(target)))
+		AsLinkableLeaf(symlink), virtual.StatusOK
 }
 
 func (d *casDirectory) resolveHandle(r io.ByteReader) (virtual.DirectoryChild, virtual.Status) {
@@ -73,6 +79,7 @@ func (d *casDirectory) resolveHandle(r io.ByteReader) (virtual.DirectoryChild, v
 	if index == 0 {
 		return virtual.DirectoryChild{}.FromDirectory(d.createSelf()), virtual.StatusOK
 	}
+
 	// If a suffix is provided, it corresponds to a symbolic link
 	// inside this directory. Files and directories will be
 	// resolvable through other means.
@@ -84,7 +91,11 @@ func (d *casDirectory) resolveHandle(r io.ByteReader) (virtual.DirectoryChild, v
 	if index >= uint64(len(directory.Symlinks)) {
 		return virtual.DirectoryChild{}, virtual.StatusErrBadHandle
 	}
-	return virtual.DirectoryChild{}.FromLeaf(d.createSymlink(index, directory.Symlinks[index].Target)), virtual.StatusOK
+	symlink, s := d.createSymlink(index, directory.Symlinks[index].Target)
+	if s != virtual.StatusOK {
+		return virtual.DirectoryChild{}, s
+	}
+	return virtual.DirectoryChild{}.FromLeaf(symlink), virtual.StatusOK
 }
 
 func (d *casDirectory) VirtualGetAttributes(ctx context.Context, requested virtual.AttributesMask, attributes *virtual.Attributes) {
@@ -137,7 +148,10 @@ func (d *casDirectory) VirtualLookup(ctx context.Context, name path.Component, r
 
 	symlinks := directory.Symlinks
 	if i := sort.Search(len(symlinks), func(i int) bool { return symlinks[i].Name >= n }); i < len(symlinks) && symlinks[i].Name == n {
-		f := d.createSymlink(uint64(i), symlinks[i].Target)
+		f, s := d.createSymlink(uint64(i), symlinks[i].Target)
+		if s != virtual.StatusOK {
+			return virtual.DirectoryChild{}, s
+		}
 		f.VirtualGetAttributes(ctx, requested, out)
 		return virtual.DirectoryChild{}.FromLeaf(f), virtual.StatusOK
 	}
@@ -242,7 +256,10 @@ func (d *casDirectory) VirtualReadDir(ctx context.Context, firstCookie uint64, r
 			d.directoryContext.LogError(status.Errorf(codes.InvalidArgument, "Symbolic link %#v has an invalid name", entry.Name))
 			return virtual.StatusErrIO
 		}
-		child := d.createSymlink(i, entry.Target)
+		child, s := d.createSymlink(i, entry.Target)
+		if s != virtual.StatusOK {
+			return s
+		}
 		var attributes virtual.Attributes
 		child.VirtualGetAttributes(ctx, requested, &attributes)
 		if !reporter.ReportEntry(nextCookieOffset+i, name, virtual.DirectoryChild{}.FromLeaf(child), &attributes) {

pkg/filesystem/virtual/cas_directory_test.go

@@ -236,9 +236,15 @@ func TestCASDirectoryVirtualLookup(t *testing.T) {
 		require.Equal(t, filesystem.FileTypeSymlink, out.GetFileType())
 
 		_, actualLeaf := actualChild.GetPair()
-		target, s := actualLeaf.VirtualReadlink(ctx)
-		require.Equal(t, re_vfs.StatusOK, s)
-		require.Equal(t, []byte("target"), target)
+		var attributes re_vfs.Attributes
+		actualLeaf.VirtualGetAttributes(ctx, re_vfs.AttributesMaskSymlinkTarget, &attributes)
+
+		symlinkTarget, ok := attributes.GetSymlinkTarget()
+		require.True(t, ok)
+
+		symlinkTargetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
+		require.NoError(t, path.Resolve(symlinkTarget, scopeWalker))
+		require.Equal(t, "target", symlinkTargetBuilder.GetUNIXString())
 	})
 }
 
@@ -587,8 +593,14 @@ func TestCASDirectoryHandleResolver(t *testing.T) {
 		require.Equal(t, re_vfs.StatusOK, s)
 
 		_, leaf := child.GetPair()
-		target, s := leaf.VirtualReadlink(ctx)
-		require.Equal(t, re_vfs.StatusOK, s)
-		require.Equal(t, []byte("target2"), target)
+		var attributes re_vfs.Attributes
+		leaf.VirtualGetAttributes(ctx, re_vfs.AttributesMaskSymlinkTarget, &attributes)
+
+		symlinkTarget, ok := attributes.GetSymlinkTarget()
+		require.True(t, ok)
+
+		symlinkTargetBuilder, scopeWalker := path.EmptyBuilder.Join(path.VoidScopeWalker)
+		require.NoError(t, path.Resolve(symlinkTarget, scopeWalker))
+		require.Equal(t, "target2", symlinkTargetBuilder.GetUNIXString())
 	})
 }

pkg/filesystem/virtual/persistent_output_path_factory.go

@@ -100,7 +100,14 @@ func (sr *stateRestorer) restoreDirectoryRecursive(reader outputpathpersistency.
 			return status.Errorf(codes.InvalidArgument, "Directory contains multiple children named %#v", entry.Name)
 		}
 
-		initialNodes[component] = virtual.InitialChild{}.FromLeaf(sr.symlinkFactory.LookupSymlink([]byte(entry.Target)))
+		// TODO: Should this use path.LocalFormat? If so,
+		// ApplyAppendOutputPathPersistencyDirectoryNode should also be
+		// modified to take a path.Format.
+		symlinkNode, err := sr.symlinkFactory.LookupSymlink(path.UNIXFormat.NewParser(entry.Target))
+		if err != nil {
+			return status.Errorf(codes.InvalidArgument, "Failed to create symbolic link %#v with target %#v", entry.Name, entry.Target)
+		}
+		initialNodes[component] = virtual.InitialChild{}.FromLeaf(symlinkNode)
 	}
 	if err := d.CreateChildren(initialNodes, true); err != nil {
 		return util.StatusWrap(err, "Failed to create files and symbolic links")

pkg/filesystem/virtual/persistent_output_path_factory_test.go

@@ -230,7 +230,7 @@ func TestPersistentOutputPathFactoryStartInitialBuild(t *testing.T) {
 			/* readMonitor = */ nil,
 		).Return(file2)
 		symlink1 := mock.NewMockLinkableLeaf(ctrl)
-		symlinkFactory.EXPECT().LookupSymlink([]byte("target1")).Return(symlink1)
+		symlinkFactory.EXPECT().LookupSymlink(path.UNIXFormat.NewParser("target1")).Return(symlink1, nil)
 		baseOutputPath.EXPECT().CreateChildren(map[path.Component]re_vfs.InitialChild{
 			path.MustNewComponent("file1"):    re_vfs.InitialChild{}.FromLeaf(file1),
 			path.MustNewComponent("file2"):    re_vfs.InitialChild{}.FromLeaf(file2),