Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace GetContainingDigests() with VirtualApply() #155

Merged
merged 3 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions pkg/builder/virtual_build_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,8 @@ func (d *virtualBuildDirectory) Lstat(name path.Component) (filesystem.FileInfo,
}

func (d *virtualBuildDirectory) Mkdir(name path.Component, mode os.FileMode) error {
return d.CreateChildren(map[path.Component]virtual.InitialNode{
name: virtual.InitialNode{}.FromDirectory(virtual.EmptyInitialContentsFetcher),
return d.CreateChildren(map[path.Component]virtual.InitialChild{
name: virtual.InitialChild{}.FromDirectory(virtual.EmptyInitialContentsFetcher),
}, false)
}

Expand All @@ -150,8 +150,8 @@ func (d *virtualBuildDirectory) Mknod(name path.Component, perm os.FileMode, dev
return status.Error(codes.InvalidArgument, "The provided file mode is not for a character device")
}
characterDevice := d.options.characterDeviceFactory.LookupCharacterDevice(deviceNumber)
if err := d.CreateChildren(map[path.Component]virtual.InitialNode{
name: virtual.InitialNode{}.FromLeaf(characterDevice),
if err := d.CreateChildren(map[path.Component]virtual.InitialChild{
name: virtual.InitialChild{}.FromLeaf(characterDevice),
}, false); err != nil {
characterDevice.Unlink()
return err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func NewAccessMonitoringInitialContentsFetcher(base InitialContentsFetcher, root
}
}

func (icf *accessMonitoringInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialNode, error) {
func (icf *accessMonitoringInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialChild, error) {
// Call into underlying initial contents fetcher. Wrap the file
// read monitors that are installed on the files, so that we can
// detect file access.
Expand All @@ -43,16 +43,16 @@ func (icf *accessMonitoringInitialContentsFetcher) FetchContents(fileReadMonitor

// Wrap all of the child directories, so that we can detect
// directory access.
wrappedContents := make(map[path.Component]InitialNode, len(contents))
wrappedContents := make(map[path.Component]InitialChild, len(contents))
for name, node := range contents {
childInitialContentsFetcher, leaf := node.GetPair()
if childInitialContentsFetcher != nil {
wrappedContents[name] = InitialNode{}.FromDirectory(&accessMonitoringInitialContentsFetcher{
wrappedContents[name] = InitialChild{}.FromDirectory(&accessMonitoringInitialContentsFetcher{
InitialContentsFetcher: childInitialContentsFetcher,
unreadDirectoryMonitor: readDirectoryMonitor.ResolvedDirectory(name),
})
} else {
wrappedContents[name] = InitialNode{}.FromLeaf(leaf)
wrappedContents[name] = InitialChild{}.FromLeaf(leaf)
}
}
return wrappedContents, nil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ func TestAccessMonitoringInitialContentsFetcher(t *testing.T) {
baseFileReadMonitorFactory.EXPECT().Call(path.MustNewComponent("file")).Return(baseChildFileReadMonitor.Call)
var childFileReadMonitor virtual.FileReadMonitor
baseInitialContentsFetcher.EXPECT().FetchContents(gomock.Any()).
DoAndReturn(func(fileReadMonitorFactory virtual.FileReadMonitorFactory) (map[path.Component]virtual.InitialNode, error) {
DoAndReturn(func(fileReadMonitorFactory virtual.FileReadMonitorFactory) (map[path.Component]virtual.InitialChild, error) {
childFileReadMonitor = fileReadMonitorFactory(path.MustNewComponent("file"))
return map[path.Component]virtual.InitialNode{
path.MustNewComponent("dir"): virtual.InitialNode{}.FromDirectory(baseChildInitialContentsFetcher),
path.MustNewComponent("file"): virtual.InitialNode{}.FromLeaf(baseChildFile),
return map[path.Component]virtual.InitialChild{
path.MustNewComponent("dir"): virtual.InitialChild{}.FromDirectory(baseChildInitialContentsFetcher),
path.MustNewComponent("file"): virtual.InitialChild{}.FromLeaf(baseChildFile),
}, nil
})
rootReadDirectoryMonitor := mock.NewMockReadDirectoryMonitor(ctrl)
Expand All @@ -69,7 +69,7 @@ func TestAccessMonitoringInitialContentsFetcher(t *testing.T) {
childInitialContentsFetcher, _ := rootContents[path.MustNewComponent("dir")].GetPair()

t.Run("FetchContentsSucceeded", func(t *testing.T) {
baseChildInitialContentsFetcher.EXPECT().FetchContents(gomock.Any()).Return(map[path.Component]virtual.InitialNode{}, nil)
baseChildInitialContentsFetcher.EXPECT().FetchContents(gomock.Any()).Return(map[path.Component]virtual.InitialChild{}, nil)
childReadDirectoryMonitor := mock.NewMockReadDirectoryMonitor(ctrl)
childUnreadDirectoryMonitor.EXPECT().ReadDirectory().Return(childReadDirectoryMonitor)
baseChildFileReadMonitorFactory := mock.NewMockFileReadMonitorFactory(ctrl)
Expand Down
44 changes: 25 additions & 19 deletions pkg/filesystem/virtual/cas_initial_contents_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func NewCASInitialContentsFetcher(ctx context.Context, directoryWalker cas.Direc
}
}

func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialNode, error) {
func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialChild, error) {
directory, err := icf.directoryWalker.GetDirectory(icf.options.context)
if err != nil {
return nil, err
Expand All @@ -53,7 +53,7 @@ func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFact
// Create InitialContentsFetchers for all child directories.
// These can yield even more InitialContentsFetchers for
// grandchildren.
children := make(map[path.Component]InitialNode, len(directory.Directories)+len(directory.Files)+len(directory.Symlinks))
children := make(map[path.Component]InitialChild, len(directory.Directories)+len(directory.Files)+len(directory.Symlinks))
for _, entry := range directory.Directories {
component, ok := path.NewComponent(entry.Name)
if !ok {
Expand All @@ -67,7 +67,7 @@ func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFact
if err != nil {
return nil, util.StatusWrapf(err, "Failed to obtain digest for directory %#v", entry.Name)
}
children[component] = InitialNode{}.FromDirectory(&casInitialContentsFetcher{
children[component] = InitialChild{}.FromDirectory(&casInitialContentsFetcher{
options: icf.options,
directoryWalker: icf.directoryWalker.GetChild(childDigest),
})
Expand Down Expand Up @@ -96,7 +96,7 @@ func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFact
return nil, util.StatusWrapf(err, "Failed to obtain digest for file %#v", entry.Name)
}
leaf := icf.options.casFileFactory.LookupFile(childDigest, entry.IsExecutable, fileReadMonitorFactory(component))
children[component] = InitialNode{}.FromLeaf(leaf)
children[component] = InitialChild{}.FromLeaf(leaf)
leavesToUnlink = append(leavesToUnlink, leaf)
}

Expand All @@ -111,39 +111,45 @@ func (icf *casInitialContentsFetcher) fetchContentsUnwrapped(fileReadMonitorFact
}

leaf := icf.options.symlinkFactory.LookupSymlink([]byte(entry.Target))
children[component] = InitialNode{}.FromLeaf(leaf)
children[component] = InitialChild{}.FromLeaf(leaf)
leavesToUnlink = append(leavesToUnlink, leaf)
}

leavesToUnlink = nil
return children, nil
}

func (icf *casInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialNode, error) {
func (icf *casInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialChild, error) {
children, err := icf.fetchContentsUnwrapped(fileReadMonitorFactory)
if err != nil {
return nil, util.StatusWrap(err, icf.directoryWalker.GetDescription())
}
return children, nil
}

func (icf *casInitialContentsFetcher) GetContainingDigests(ctx context.Context) (digest.Set, error) {
gatherer := casContainingDigestsGatherer{
context: ctx,
digestFunction: icf.options.digestFunction,
digests: digest.NewSetBuilder(),
directoriesGathered: map[digest.Digest]struct{}{},
}
err := gatherer.traverse(icf.directoryWalker)
if err != nil {
return digest.EmptySet, err
func (icf *casInitialContentsFetcher) VirtualApply(data any) bool {
switch p := data.(type) {
case *ApplyGetContainingDigests:
gatherer := casContainingDigestsGatherer{
context: p.Context,
digestFunction: icf.options.digestFunction,
digests: digest.NewSetBuilder(),
directoriesGathered: map[digest.Digest]struct{}{},
}
if err := gatherer.traverse(icf.directoryWalker); err == nil {
p.ContainingDigests = gatherer.digests.Build()
} else {
p.Err = err
}
default:
return false
}
return gatherer.digests.Build(), nil
return true
}

// casContainingDigestsGatherer is used by casInitialContentsFetcher's
// GetContainingDigests() to compute the transitive closure of digests
// referenced by a hierarchy of Directory objects.
// implementation of ApplyGetContainingDigests to compute the transitive
// closure of digests referenced by a hierarchy of Directory objects.
type casContainingDigestsGatherer struct {
context context.Context
digestFunction digest.Function
Expand Down
40 changes: 26 additions & 14 deletions pkg/filesystem/virtual/cas_initial_contents_fetcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,11 @@ func TestCASInitialContentsFetcherFetchContents(t *testing.T) {
children, err := initialContentsFetcher.FetchContents(fileReadMonitorFactory.Call)
require.NoError(t, err)
childInitialContentsFetcher, _ := children[path.MustNewComponent("directory")].GetPair()
require.Equal(t, map[path.Component]virtual.InitialNode{
path.MustNewComponent("directory"): virtual.InitialNode{}.FromDirectory(childInitialContentsFetcher),
path.MustNewComponent("executable"): virtual.InitialNode{}.FromLeaf(executableLeaf),
path.MustNewComponent("file"): virtual.InitialNode{}.FromLeaf(fileLeaf),
path.MustNewComponent("symlink"): virtual.InitialNode{}.FromLeaf(symlinkLeaf),
require.Equal(t, map[path.Component]virtual.InitialChild{
path.MustNewComponent("directory"): virtual.InitialChild{}.FromDirectory(childInitialContentsFetcher),
path.MustNewComponent("executable"): virtual.InitialChild{}.FromLeaf(executableLeaf),
path.MustNewComponent("file"): virtual.InitialChild{}.FromLeaf(fileLeaf),
path.MustNewComponent("symlink"): virtual.InitialChild{}.FromLeaf(symlinkLeaf),
}, children)

// Check that the InitialContentsFetcher that is created
Expand Down Expand Up @@ -257,8 +257,11 @@ func TestCASInitialContentsFetcherGetContainingDigests(t *testing.T) {
Return(nil, status.Error(codes.Internal, "Server failure"))
directoryWalker.EXPECT().GetDescription().Return("Root directory")

_, err := initialContentsFetcher.GetContainingDigests(ctx)
testutil.RequireEqualStatus(t, status.Error(codes.Internal, "Root directory: Server failure"), err)
p := virtual.ApplyGetContainingDigests{
Context: ctx,
}
require.True(t, initialContentsFetcher.VirtualApply(&p))
testutil.RequireEqualStatus(t, status.Error(codes.Internal, "Root directory: Server failure"), p.Err)
})

t.Run("ChildDirectoryInvalidDigest", func(t *testing.T) {
Expand All @@ -277,8 +280,11 @@ func TestCASInitialContentsFetcherGetContainingDigests(t *testing.T) {
}, nil)
directoryWalker.EXPECT().GetDescription().Return("Root directory")

_, err := initialContentsFetcher.GetContainingDigests(ctx)
testutil.RequireEqualStatus(t, status.Error(codes.InvalidArgument, "Root directory: Failed to obtain digest for directory \"hello\": Hash has length 18, while 32 characters were expected"), err)
p := virtual.ApplyGetContainingDigests{
Context: ctx,
}
require.True(t, initialContentsFetcher.VirtualApply(&p))
testutil.RequireEqualStatus(t, status.Error(codes.InvalidArgument, "Root directory: Failed to obtain digest for directory \"hello\": Hash has length 18, while 32 characters were expected"), p.Err)
})

t.Run("ChildFileInvalidDigest", func(t *testing.T) {
Expand All @@ -297,8 +303,11 @@ func TestCASInitialContentsFetcherGetContainingDigests(t *testing.T) {
}, nil)
directoryWalker.EXPECT().GetDescription().Return("Root directory")

_, err := initialContentsFetcher.GetContainingDigests(ctx)
testutil.RequireEqualStatus(t, status.Error(codes.InvalidArgument, "Root directory: Failed to obtain digest for file \"hello\": Hash has length 18, while 32 characters were expected"), err)
p := virtual.ApplyGetContainingDigests{
Context: ctx,
}
require.True(t, initialContentsFetcher.VirtualApply(&p))
testutil.RequireEqualStatus(t, status.Error(codes.InvalidArgument, "Root directory: Failed to obtain digest for file \"hello\": Hash has length 18, while 32 characters were expected"), p.Err)
})

t.Run("Success", func(t *testing.T) {
Expand Down Expand Up @@ -357,8 +366,11 @@ func TestCASInitialContentsFetcherGetContainingDigests(t *testing.T) {
},
}, nil)

digests, err := initialContentsFetcher.GetContainingDigests(ctx)
require.NoError(t, err)
p := virtual.ApplyGetContainingDigests{
Context: ctx,
}
require.True(t, initialContentsFetcher.VirtualApply(&p))
require.NoError(t, p.Err)
require.Equal(
t,
digest.NewSetBuilder().
Expand All @@ -367,6 +379,6 @@ func TestCASInitialContentsFetcherGetContainingDigests(t *testing.T) {
Add(digest.MustNewDigest("hello", remoteexecution.DigestFunction_MD5, "c0607941dd5b3ca8e175a1bfbfd1c0ea", 789)).
Add(digest.MustNewDigest("hello", remoteexecution.DigestFunction_MD5, "19dc69325bd8dfcd75cefbb6144ea3bb", 42)).
Build(),
digests)
p.ContainingDigests)
})
}
11 changes: 4 additions & 7 deletions pkg/filesystem/virtual/empty_initial_contents_fetcher.go
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package virtual

import (
"context"

"github.com/buildbarn/bb-storage/pkg/digest"
"github.com/buildbarn/bb-storage/pkg/filesystem/path"
)

type emptyInitialContentsFetcher struct{}

func (f emptyInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialNode, error) {
return map[path.Component]InitialNode{}, nil
func (f emptyInitialContentsFetcher) FetchContents(fileReadMonitorFactory FileReadMonitorFactory) (map[path.Component]InitialChild, error) {
return map[path.Component]InitialChild{}, nil
}

func (f emptyInitialContentsFetcher) GetContainingDigests(ctx context.Context) (digest.Set, error) {
return digest.EmptySet, nil
func (f emptyInitialContentsFetcher) VirtualApply(data any) bool {
return false
}

// EmptyInitialContentsFetcher is an instance of InitialContentsFetcher
Expand Down
8 changes: 4 additions & 4 deletions pkg/filesystem/virtual/in_memory_prepopulated_directory.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func (c *inMemoryDirectoryContents) isDeletable(hiddenFilesMatcher StringMatcher
return true
}

func (c *inMemoryDirectoryContents) createChildren(subtree *inMemorySubtree, children map[path.Component]InitialNode) {
func (c *inMemoryDirectoryContents) createChildren(subtree *inMemorySubtree, children map[path.Component]InitialChild) {
// Either sort or shuffle the children before inserting them
// into the directory. This either makes VirtualReadDir() behave
// deterministically, or not, based on preference.
Expand Down Expand Up @@ -519,7 +519,7 @@ func (i *inMemoryPrepopulatedDirectory) InstallHooks(fileAllocator FileAllocator
}
}

func (i *inMemoryPrepopulatedDirectory) CreateChildren(children map[path.Component]InitialNode, overwrite bool) error {
func (i *inMemoryPrepopulatedDirectory) CreateChildren(children map[path.Component]InitialChild, overwrite bool) error {
i.lock.Lock()
contents, err := i.getContents()
if err != nil {
Expand Down Expand Up @@ -598,7 +598,7 @@ func (i *inMemoryPrepopulatedDirectory) filterChildrenRecursive(childFilter Chil
// instantiate it. Simply provide the
// InitialContentsFetcher to the callback.
i.lock.Unlock()
return childFilter(InitialNode{}.FromDirectory(initialContentsFetcher), func() error {
return childFilter(InitialChild{}.FromDirectory(initialContentsFetcher), func() error {
return i.RemoveAllChildren(false)
})
}
Expand Down Expand Up @@ -626,7 +626,7 @@ func (i *inMemoryPrepopulatedDirectory) filterChildrenRecursive(childFilter Chil
// Invoke the callback for all children.
for _, child := range leaves {
name := child.name
if !childFilter(InitialNode{}.FromLeaf(child.leaf), func() error {
if !childFilter(InitialChild{}.FromLeaf(child.leaf), func() error {
return i.Remove(name)
}) {
return false
Expand Down
Loading
Loading