Skip to content

Commit 2c64858

Browse files
EdSchoutenEdSchouten
committed
Make ctx.actions.symlink(target_file=...) more complete
- Keep track of whether is_executable=True is set. - Allow placing them in runfiles directories.
1 parent f3f756c commit 2c64858

8 files changed

Lines changed: 907 additions & 609 deletions

File tree

‎pkg/model/analysis/configured_target.go‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2367,7 +2367,7 @@ func (rca *ruleContextActions[TReference, TMetadata]) doRun(thread *starlark.Thr
23672367
}
23682368
}
23692369

2370-
argv0, err = model_starlark.FileGetPath(executableFile.GetDefinition(), nil)
2370+
argv0, err = model_starlark.FileGetInputRootPath(executableFile.GetDefinition(), nil)
23712371
if err != nil {
23722372
return nil, fmt.Errorf("executable: %w", err)
23732373
}
@@ -2808,6 +2808,9 @@ func (rca *ruleContextActions[TReference, TMetadata]) doSymlink(thread *starlark
28082808
if useExecRootForSource {
28092809
return nil, errors.New("this implementation does not support use_exec_root_for_source=True")
28102810
}
2811+
if isExecutable && output.fileType != model_starlark_pb.File_FILE {
2812+
return nil, errors.New("is_executable=True can only be used in combination with regular file outputs")
2813+
}
28112814

28122815
if targetFile != nil {
28132816
if targetPath != nil {
@@ -2825,8 +2828,11 @@ func (rca *ruleContextActions[TReference, TMetadata]) doSymlink(thread *starlark
28252828
return starlark.None, output.setDefinition(
28262829
model_core.NewPatchedMessage(
28272830
&model_analysis_pb.TargetOutputDefinition{
2828-
Source: &model_analysis_pb.TargetOutputDefinition_Symlink{
2829-
Symlink: patchedTargetFileDefinition.Message,
2831+
Source: &model_analysis_pb.TargetOutputDefinition_Symlink_{
2832+
Symlink: &model_analysis_pb.TargetOutputDefinition_Symlink{
2833+
Target: patchedTargetFileDefinition.Message,
2834+
IsExecutable: isExecutable,
2835+
},
28302836
},
28312837
},
28322838
patchedTargetFileDefinition.Patcher,

‎pkg/model/analysis/file_root.go‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ func getStarlarkFileProperties[TReference object.BasicReference, TMetadata model
5757
return model_core.Message[*model_filesystem_pb.FileProperties, TReference]{}, evaluation.ErrMissingDependency
5858
}
5959

60-
filePath, err := model_starlark.FileGetPath(f, nil)
60+
filePath, err := model_starlark.FileGetInputRootPath(f, nil)
6161
if err != nil {
6262
return model_core.Message[*model_filesystem_pb.FileProperties, TReference]{}, err
6363
}
@@ -114,6 +114,17 @@ func getPackageOutputDirectoryComponents[TReference object.BasicReference](confi
114114
return components, nil
115115
}
116116

117+
func fileGetPathInDirectoryLayout[TReference object.BasicReference](f model_core.Message[*model_starlark_pb.File, TReference], directoryLayout model_analysis_pb.DirectoryLayout) (string, error) {
118+
switch directoryLayout {
119+
case model_analysis_pb.DirectoryLayout_INPUT_ROOT:
120+
return model_starlark.FileGetInputRootPath(f, nil)
121+
case model_analysis_pb.DirectoryLayout_RUNFILES:
122+
return model_starlark.FileGetRunfilesPath(f)
123+
default:
124+
return "", errors.New("unknown directory layout")
125+
}
126+
}
127+
117128
func (c *baseComputer[TReference, TMetadata]) ComputeFileRootValue(ctx context.Context, key model_core.Message[*model_analysis_pb.FileRoot_Key, TReference], e FileRootEnvironment[TReference, TMetadata]) (PatchedFileRootValue, error) {
118129
f := model_core.Nested(key, key.Message.File)
119130
if f.Message == nil {
@@ -378,16 +389,12 @@ func (c *baseComputer[TReference, TMetadata]) ComputeFileRootValue(ctx context.C
378389
},
379390
model_core.MapReferenceMetadataToWalkers(createdDirectory.Message.Patcher),
380391
), nil
381-
case *model_analysis_pb.TargetOutputDefinition_Symlink:
382-
if key.Message.DirectoryLayout != model_analysis_pb.DirectoryLayout_INPUT_ROOT {
383-
return PatchedFileRootValue{}, errors.New("TODO: Support symlinks with runfiles layout")
384-
}
385-
392+
case *model_analysis_pb.TargetOutputDefinition_Symlink_:
386393
// Symlink to another file. Obtain the root of
387394
// the target and add a symlink to it.
388395
directoryCreationParameters, gotDirectoryCreationParameters := e.GetDirectoryCreationParametersObjectValue(&model_analysis_pb.DirectoryCreationParametersObject_Key{})
389396
directoryReaders, gotDirectoryReaders := e.GetDirectoryReadersValue(&model_analysis_pb.DirectoryReaders_Key{})
390-
symlinkTargetFile := model_core.Nested(output, source.Symlink)
397+
symlinkTargetFile := model_core.Nested(output, source.Symlink.Target)
391398
patchedSymlinkTargetFile := model_core.Patch(e, symlinkTargetFile)
392399
symlinkTarget := e.GetFileRootValue(
393400
model_core.NewPatchedMessage(
@@ -402,18 +409,18 @@ func (c *baseComputer[TReference, TMetadata]) ComputeFileRootValue(ctx context.C
402409
return PatchedFileRootValue{}, evaluation.ErrMissingDependency
403410
}
404411

412+
symlinkPath, err := fileGetPathInDirectoryLayout(f, key.Message.DirectoryLayout)
413+
if err != nil {
414+
return PatchedFileRootValue{}, err
415+
}
416+
405417
rootDirectory := changeTrackingDirectory[TReference, TMetadata]{
406418
unmodifiedDirectory: model_core.Nested(symlinkTarget, &model_filesystem_pb.Directory{
407419
Contents: &model_filesystem_pb.Directory_ContentsInline{
408420
ContentsInline: symlinkTarget.Message.RootDirectory,
409421
},
410422
}),
411423
}
412-
413-
symlinkPath, err := model_starlark.FileGetPath(f, nil)
414-
if err != nil {
415-
return PatchedFileRootValue{}, err
416-
}
417424
loadOptions := &changeTrackingDirectoryLoadOptions[TReference]{
418425
context: ctx,
419426
directoryContentsReader: directoryReaders.DirectoryContents,
@@ -434,7 +441,7 @@ func (c *baseComputer[TReference, TMetadata]) ComputeFileRootValue(ctx context.C
434441
// the directory in which it is contained.
435442
// Remove leading components of the target that
436443
// are equal to those of the symlink's path.
437-
targetPath, err := model_starlark.FileGetPath(symlinkTargetFile, nil)
444+
targetPath, err := fileGetPathInDirectoryLayout(symlinkTargetFile, key.Message.DirectoryLayout)
438445
if err != nil {
439446
return PatchedFileRootValue{}, err
440447
}
@@ -451,6 +458,8 @@ func (c *baseComputer[TReference, TMetadata]) ComputeFileRootValue(ctx context.C
451458
return PatchedFileRootValue{}, fmt.Errorf("failed to create symlink at %#v: %w", symlinkPath, err)
452459
}
453460

461+
// TODO: Validate IsExecutable!
462+
454463
return createFileRootFromChangeTrackingDirectory(
455464
ctx,
456465
e,

‎pkg/model/analysis/file_root_test.go‎

Lines changed: 210 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,24 @@ var emptyLeaves = &model_filesystem_pb.DirectoryContents_LeavesInline{
2222
LeavesInline: &model_filesystem_pb.Leaves{},
2323
}
2424

25+
func directoryNode(name string, contents *model_filesystem_pb.DirectoryContents) *model_filesystem_pb.DirectoryNode {
26+
return &model_filesystem_pb.DirectoryNode{
27+
Name: name,
28+
Directory: &model_filesystem_pb.Directory{
29+
Contents: &model_filesystem_pb.Directory_ContentsInline{
30+
ContentsInline: contents,
31+
},
32+
},
33+
}
34+
}
35+
2536
// singleChildDirectoryContents can be used to construct a directory
2637
// that only contains a single child that is also a directory.
2738
func singleChildDirectoryContents(name string, childContents *model_filesystem_pb.DirectoryContents) *model_filesystem_pb.DirectoryContents {
2839
return &model_filesystem_pb.DirectoryContents{
29-
Directories: []*model_filesystem_pb.DirectoryNode{{
30-
Name: name,
31-
Directory: &model_filesystem_pb.Directory{
32-
Contents: &model_filesystem_pb.Directory_ContentsInline{
33-
ContentsInline: childContents,
34-
},
35-
},
36-
}},
40+
Directories: []*model_filesystem_pb.DirectoryNode{
41+
directoryNode(name, childContents),
42+
},
3743
Leaves: emptyLeaves,
3844
}
3945
}
@@ -674,6 +680,201 @@ func TestFileRoot(t *testing.T) {
674680
})
675681

676682
t.Run("Symlink", func(t *testing.T) {
677-
// TODO!
683+
// TODO: Test error cases.
684+
685+
t.Run("Success", func(t *testing.T) {
686+
// Simulate the computation of the output of:
687+
//
688+
// output = ctx.actions.declare_file("b")
689+
// ctx.actions.symlink(
690+
// output = output,
691+
// target_file = File("@@myrepo+//:a"),
692+
// )
693+
run := func(t *testing.T, directoryLayout model_analysis_pb.DirectoryLayout, target *model_filesystem_pb.DirectoryContents) model_analysis.PatchedFileRootValue {
694+
e := NewMockFileRootEnvironmentForTesting(ctrl)
695+
bct.expectCaptureExistingObject(e)
696+
bct.expectGetDirectoryCreationParametersObjectValue(t, e)
697+
bct.expectGetDirectoryReadersValue(t, e)
698+
e.EXPECT().GetTargetOutputValue(
699+
eqPatchedMessage(func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.TargetOutput_Key {
700+
return &model_analysis_pb.TargetOutput_Key{
701+
Label: "@@myrepo+//:create_symlink",
702+
ConfigurationReference: attachObject(patcher, exampleConfiguration),
703+
PackageRelativePath: "b",
704+
}
705+
}),
706+
).Return(newMessage(func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.TargetOutput_Value {
707+
return &model_analysis_pb.TargetOutput_Value{
708+
Definition: &model_analysis_pb.TargetOutputDefinition{
709+
Source: &model_analysis_pb.TargetOutputDefinition_Symlink_{
710+
Symlink: &model_analysis_pb.TargetOutputDefinition_Symlink{
711+
Target: &model_starlark_pb.File{
712+
Label: "@@myrepo+//:a",
713+
Type: model_starlark_pb.File_FILE,
714+
},
715+
},
716+
},
717+
},
718+
}
719+
}))
720+
e.EXPECT().GetFileRootValue(
721+
eqPatchedMessage(func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.FileRoot_Key {
722+
return &model_analysis_pb.FileRoot_Key{
723+
DirectoryLayout: directoryLayout,
724+
File: &model_starlark_pb.File{
725+
Label: "@@myrepo+//:a",
726+
Type: model_starlark_pb.File_FILE,
727+
},
728+
}
729+
}),
730+
).Return(newMessage(func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.FileRoot_Value {
731+
return &model_analysis_pb.FileRoot_Value{
732+
RootDirectory: target,
733+
}
734+
}))
735+
736+
fileRoot, err := bct.computer.ComputeFileRootValue(
737+
ctx,
738+
newMessage(func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.FileRoot_Key {
739+
return &model_analysis_pb.FileRoot_Key{
740+
DirectoryLayout: directoryLayout,
741+
File: &model_starlark_pb.File{
742+
Label: "@@myrepo+//:b",
743+
Type: model_starlark_pb.File_FILE,
744+
Owner: &model_starlark_pb.File_Owner{
745+
ConfigurationReference: attachObject(patcher, exampleConfiguration),
746+
TargetName: "create_symlink",
747+
},
748+
},
749+
}
750+
}),
751+
e,
752+
)
753+
require.NoError(t, err)
754+
return fileRoot
755+
}
756+
757+
t.Run("InputRoot", func(t *testing.T) {
758+
fileRoot := run(
759+
t,
760+
model_analysis_pb.DirectoryLayout_INPUT_ROOT,
761+
singleChildDirectoryContents(
762+
"external",
763+
singleChildDirectoryContents(
764+
"myrepo+",
765+
&model_filesystem_pb.DirectoryContents{
766+
Leaves: &model_filesystem_pb.DirectoryContents_LeavesInline{
767+
LeavesInline: &model_filesystem_pb.Leaves{
768+
Files: []*model_filesystem_pb.FileNode{
769+
{
770+
Name: "a",
771+
Properties: &model_filesystem_pb.FileProperties{},
772+
},
773+
},
774+
},
775+
},
776+
},
777+
),
778+
),
779+
)
780+
requireEqualPatchedMessage(t, func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.FileRoot_Value {
781+
return &model_analysis_pb.FileRoot_Value{
782+
RootDirectory: &model_filesystem_pb.DirectoryContents{
783+
Directories: []*model_filesystem_pb.DirectoryNode{
784+
directoryNode("bazel-out", singleChildDirectoryContents(
785+
"Cg6Kx80o8BPYmGdgWYfRZvbKyWojQ7snQzHOx70XAwRPAAAAAAAAAA.",
786+
singleChildDirectoryContents(
787+
"bin",
788+
singleChildDirectoryContents(
789+
"external",
790+
singleChildDirectoryContents(
791+
"myrepo+",
792+
&model_filesystem_pb.DirectoryContents{
793+
Leaves: &model_filesystem_pb.DirectoryContents_LeavesInline{
794+
LeavesInline: &model_filesystem_pb.Leaves{
795+
Symlinks: []*model_filesystem_pb.SymlinkNode{
796+
{
797+
Name: "b",
798+
Target: "../../../../../external/myrepo+/a",
799+
},
800+
},
801+
},
802+
},
803+
},
804+
),
805+
),
806+
),
807+
)),
808+
directoryNode("external", singleChildDirectoryContents(
809+
"myrepo+",
810+
&model_filesystem_pb.DirectoryContents{
811+
Leaves: &model_filesystem_pb.DirectoryContents_LeavesInline{
812+
LeavesInline: &model_filesystem_pb.Leaves{
813+
Files: []*model_filesystem_pb.FileNode{
814+
{
815+
Name: "a",
816+
Properties: &model_filesystem_pb.FileProperties{},
817+
},
818+
},
819+
},
820+
},
821+
},
822+
)),
823+
},
824+
Leaves: emptyLeaves,
825+
},
826+
}
827+
}, fileRoot)
828+
fileRoot.Discard()
829+
})
830+
831+
t.Run("Runfiles", func(t *testing.T) {
832+
fileRoot := run(
833+
t,
834+
model_analysis_pb.DirectoryLayout_RUNFILES,
835+
singleChildDirectoryContents(
836+
"myrepo+",
837+
&model_filesystem_pb.DirectoryContents{
838+
Leaves: &model_filesystem_pb.DirectoryContents_LeavesInline{
839+
LeavesInline: &model_filesystem_pb.Leaves{
840+
Files: []*model_filesystem_pb.FileNode{
841+
{
842+
Name: "a",
843+
Properties: &model_filesystem_pb.FileProperties{},
844+
},
845+
},
846+
},
847+
},
848+
},
849+
),
850+
)
851+
requireEqualPatchedMessage(t, func(patcher *model_core.ReferenceMessagePatcher[model_core.CreatedObjectTree]) *model_analysis_pb.FileRoot_Value {
852+
return &model_analysis_pb.FileRoot_Value{
853+
RootDirectory: singleChildDirectoryContents(
854+
"myrepo+",
855+
&model_filesystem_pb.DirectoryContents{
856+
Leaves: &model_filesystem_pb.DirectoryContents_LeavesInline{
857+
LeavesInline: &model_filesystem_pb.Leaves{
858+
Files: []*model_filesystem_pb.FileNode{
859+
{
860+
Name: "a",
861+
Properties: &model_filesystem_pb.FileProperties{},
862+
},
863+
},
864+
Symlinks: []*model_filesystem_pb.SymlinkNode{
865+
{
866+
Name: "b",
867+
Target: "a",
868+
},
869+
},
870+
},
871+
},
872+
},
873+
),
874+
}
875+
}, fileRoot)
876+
fileRoot.Discard()
877+
})
878+
})
678879
})
679880
}

‎pkg/model/analysis/target_action_command.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ func expandFileIfDirectory[TReference object.BasicReference, TMetadata BaseCompu
215215

216216
// Traverse to the root of the directory for which files need to
217217
// be reported.
218-
directoryPath, err := model_starlark.FileGetPath(fileDefinition, nil)
218+
directoryPath, err := model_starlark.FileGetInputRootPath(fileDefinition, nil)
219219
if err != nil {
220220
*errOut = err
221221
return func(yield func(*model_starlark.File[TReference, TMetadata]) bool) {}
@@ -461,7 +461,7 @@ func (c *baseComputer[TReference, TMetadata]) ComputeTargetActionCommandValue(ct
461461
case starlark.String:
462462
s = string(typedV)
463463
case *model_starlark.File[TReference, TMetadata]:
464-
s, err = model_starlark.FileGetPath(typedV.GetDefinition(), typedV.GetTreeRelativePath())
464+
s, err = model_starlark.FileGetInputRootPath(typedV.GetDefinition(), typedV.GetTreeRelativePath())
465465
if err != nil {
466466
return PatchedTargetActionCommandValue{}, err
467467
}

‎pkg/model/analysis/target_action_input_root.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ func (c *baseComputer[TReference, TMetadata]) ComputeTargetActionInputRootValue(
123123
}
124124

125125
// Create the tool's runfiles directory.
126-
executablePath, err := model_starlark.FileGetPath(executable, nil)
126+
executablePath, err := model_starlark.FileGetInputRootPath(executable, nil)
127127
if err != nil {
128128
return PatchedTargetActionInputRootValue{}, fmt.Errorf("failed to get path of tool executable: %w", err)
129129
}

0 commit comments

Comments
 (0)