diff --git a/MODULE.bazel b/MODULE.bazel index ac49455..759ffb7 100644 --- a/MODULE.bazel +++ b/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", ) diff --git a/cmd/bb_clientd/main.go b/cmd/bb_clientd/main.go index 3dbb32a..c9b4056 100644 --- a/cmd/bb_clientd/main.go +++ b/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 diff --git a/go.mod b/go.mod index a0f3733..af2b5f0 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index a17e0f3..2b6a62a 100644 --- a/go.sum +++ b/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= diff --git a/pkg/filesystem/virtual/bazel_output_service_directory.go b/pkg/filesystem/virtual/bazel_output_service_directory.go index c75dea1..e60e139 100644 --- a/pkg/filesystem/virtual/bazel_output_service_directory.go +++ b/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,14 +552,11 @@ 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 @@ -566,7 +564,7 @@ func (cw *statWalker) OnDirectory(name path.Component) (path.GotDirectoryOrSymli 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{}, diff --git a/pkg/filesystem/virtual/bazel_output_service_directory_test.go b/pkg/filesystem/virtual/bazel_output_service_directory_test.go index 3ede013..1f9b3b9 100644 --- a/pkg/filesystem/virtual/bazel_output_service_directory_test.go +++ b/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) diff --git a/pkg/filesystem/virtual/blob_access_command_file_factory.go b/pkg/filesystem/virtual/blob_access_command_file_factory.go index 77fe2e7..8adcf07 100644 --- a/pkg/filesystem/virtual/blob_access_command_file_factory.go +++ b/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) { diff --git a/pkg/filesystem/virtual/cas_directory.go b/pkg/filesystem/virtual/cas_directory.go index 66151fe..e8047f4 100644 --- a/pkg/filesystem/virtual/cas_directory.go +++ b/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) { diff --git a/pkg/filesystem/virtual/cas_directory_test.go b/pkg/filesystem/virtual/cas_directory_test.go index c79183c..9bb1402 100644 --- a/pkg/filesystem/virtual/cas_directory_test.go +++ b/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()) }) } diff --git a/pkg/filesystem/virtual/persistent_output_path_factory.go b/pkg/filesystem/virtual/persistent_output_path_factory.go index 960db37..daef8bd 100644 --- a/pkg/filesystem/virtual/persistent_output_path_factory.go +++ b/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") diff --git a/pkg/filesystem/virtual/persistent_output_path_factory_test.go b/pkg/filesystem/virtual/persistent_output_path_factory_test.go index 72e43cc..c330452 100644 --- a/pkg/filesystem/virtual/persistent_output_path_factory_test.go +++ b/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),