Merged PR 744599: Ensure execute permissions bit is set for destination file for copy file pip scenarions

This PR covers two use cases.
Case 1 : The content source is already in the cache, so the execution of the copy file is simply materializing the content from the cache to the specified target.

Case2 : When the source of the copyfile pip is a source file.

Case3: When the source of the copy file pip is a symlink.

Previous draft PR
https://dev.azure.com/mseng/Domino/_git/BuildXL.Internal/pullrequest/744302?path=/Public/Src/Utilities/Storage/FileMaterializationInfo.cs
This commit is contained in:
Sahiti Chandramouli 2023-10-16 23:50:17 +00:00
Родитель 7a2387693a
Коммит 0c35ece51c
7 изменённых файлов: 163 добавлений и 33 удалений

Просмотреть файл

@ -32,7 +32,7 @@ extends:
- template: /.azdo/linux/job-selfhost.yml@self
parameters:
ValidationName: PublicRelease
BxlExtraArgs: /q:ReleaseLinux
BxlExtraArgs: /q:ReleaseLinux /forceAddExecutionPermission-
- stage: Build_Internal
displayName: Build (internal)
@ -43,7 +43,7 @@ extends:
parameters:
Distributed: true
ValidationName: InternalRelease
BxlExtraArgs: --internal /q:ReleaseLinux
BxlExtraArgs: --internal /q:ReleaseLinux /forceAddExecutionPermission-
- stage: Verify_PTrace
displayName: PTrace validation

Просмотреть файл

@ -695,7 +695,7 @@ namespace BuildXL.Scheduler.Artifacts
public async Task<bool> TryLoadAvailableOutputContentAsync(
PipInfo pipInfo,
OperationContext operationContext,
IReadOnlyList<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot)> filesAndContentHashes,
IReadOnlyList<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot, bool isExecutable)> filesAndContentHashes,
Action onFailure = null,
Action<int, string, string, Failure> onContentUnavailable = null,
bool materialize = false)
@ -711,7 +711,7 @@ namespace BuildXL.Scheduler.Artifacts
materializingOutputs: true,
// Only failures matter since we are checking a cache entry and not actually materializing
onlyLogUnavailableContent: true,
filesAndContentHashes: filesAndContentHashes.SelectList((tuple, index) => (tuple.fileArtifact, tuple.contentHash, index, tuple.outputDirectoryRoot)),
filesAndContentHashes: filesAndContentHashes.SelectList((tuple, index) => (tuple.fileArtifact, tuple.contentHash, index, tuple.outputDirectoryRoot, tuple.isExecutable)),
onFailure: failure => { onFailure?.Invoke(); },
onContentUnavailable: onContentUnavailable ?? ((index, expectedHash, hashOnDiskIfAvailableOrNull, failure) => { /* Do nothing. Callee already logs the failure */ }));
@ -731,7 +731,7 @@ namespace BuildXL.Scheduler.Artifacts
state.AddMaterializationFile(
fileToMaterialize: file,
allowReadOnly: true,
materializationInfo: FileMaterializationInfo.CreateWithUnknownLength(hash),
materializationInfo: FileMaterializationInfo.CreateWithUnknownLength(hash, fileAndContentHash.isExecutable),
materializationCompletion: TaskSourceSlim.Create<PipOutputOrigin>());
}
@ -2678,7 +2678,7 @@ namespace BuildXL.Scheduler.Artifacts
operationContext,
pipInfo,
state.MaterializingOutputs,
state.GetCacheMaterializationFiles().SelectList(i => (i.materializationFile.Artifact, i.materializationFile.MaterializationInfo.Hash, i.index, i.materializationFile.MaterializationInfo.OpaqueDirectoryRoot)),
state.GetCacheMaterializationFiles().SelectList(i => (i.materializationFile.Artifact, i.materializationFile.MaterializationInfo.Hash, i.index, i.materializationFile.MaterializationInfo.OpaqueDirectoryRoot, i.materializationFile.MaterializationInfo.IsExecutable)),
onFailure: failure =>
{
for (int index = 0; index < state.MaterializationFiles.Count; index++)
@ -2779,7 +2779,7 @@ namespace BuildXL.Scheduler.Artifacts
OperationContext operationContext,
PipInfo pipInfo,
bool materializingOutputs,
IReadOnlyList<(FileArtifact fileArtifact, ContentHash contentHash, int fileIndex, AbsolutePath outputDirectoryRoot)> filesAndContentHashes,
IReadOnlyList<(FileArtifact fileArtifact, ContentHash contentHash, int fileIndex, AbsolutePath outputDirectoryRoot, bool isExecutable)> filesAndContentHashes,
Action<Failure> onFailure,
Action<int, string, string, Failure> onContentUnavailable,
bool onlyLogUnavailableContent = false,
@ -2854,7 +2854,7 @@ namespace BuildXL.Scheduler.Artifacts
bool materializingOutputs,
Action<int, string, string, Failure> onContentUnavailable,
ContentAvailabilityResult result,
(FileArtifact fileArtifact, ContentHash contentHash, int fileIndex, AbsolutePath outputDirectoryRoot) filesAndContentHashesEntry,
(FileArtifact fileArtifact, ContentHash contentHash, int fileIndex, AbsolutePath outputDirectoryRoot, bool isExecutable) filesAndContentHashesEntry,
bool onlyLogUnavailableContent = false,
PipArtifactsState state = null)
{
@ -2871,6 +2871,7 @@ namespace BuildXL.Scheduler.Artifacts
var contentHash = filesAndContentHashesEntry.contentHash;
var currentFileIndex = filesAndContentHashesEntry.fileIndex;
var outputDirectoryRoot = filesAndContentHashesEntry.outputDirectoryRoot;
var isExecutable = filesAndContentHashesEntry.isExecutable;
Contract.Assume(contentHash == result.Hash);
var outerContext = operationContext.StartAsyncOperation(OperationCounter.FileContentManagerHandleContentAvailabilityOuter, fileArtifact);
@ -2926,7 +2927,8 @@ namespace BuildXL.Scheduler.Artifacts
fileArtifact,
contentHash,
fileArtifact,
outputDirectoryRoot);
outputDirectoryRoot,
isExecutable: isExecutable);
// Try to be conservative here due to distributed builds (e.g., the files may not exist on other machines).
isAvailable = possiblyStored.Succeeded;
@ -2987,7 +2989,8 @@ namespace BuildXL.Scheduler.Artifacts
otherFile,
contentHash,
fileArtifact,
otherFileOutputDirectoryRoot);
otherFileOutputDirectoryRoot,
isExecutable: isExecutable);
isAvailable = possiblyStored.Succeeded;
}
@ -3181,7 +3184,8 @@ namespace BuildXL.Scheduler.Artifacts
FileArtifact fileArtifact,
ContentHash hash,
FileArtifact targetFile,
AbsolutePath fileArtifactOutputDirectoryRoot)
AbsolutePath fileArtifactOutputDirectoryRoot,
bool isExecutable)
{
if (!Configuration.Schedule.StoreOutputsToCache && !m_host.IsFileRewritten(targetFile))
{
@ -3200,7 +3204,8 @@ namespace BuildXL.Scheduler.Artifacts
tryFlushPageCacheToFileSystem: shouldReTrack,
knownContentHash: hash,
trackPath: shouldReTrack,
outputDirectoryRoot: fileArtifactOutputDirectoryRoot);
outputDirectoryRoot: fileArtifactOutputDirectoryRoot,
isExecutable: isExecutable);
return possiblyStored;
}
@ -3507,9 +3512,12 @@ namespace BuildXL.Scheduler.Artifacts
var outputDirectoryRoot = declaredArtifact.IsDirectory && m_host.GetSealDirectoryKind(declaredArtifact.DirectoryArtifact).IsDynamicKind()
? declaredArtifact.Path
: AbsolutePath.Invalid;
// Need to pass the executable bit for the fileArtifact.
// This code path can be invoked when we try to hash the dependencies of the source files during the start step of the pip.
// In such cases we are yet to hash the source files. Hence it is not possible to get the right results using the TryGetInputContent method.
var isExecutable = FileUtilities.TryGetIsExecutableIfNeeded(fileArtifact.Path.ToString(Context.PathTable));
Possible<ContentDiscoveryResult> possiblyDiscovered =
await LocalDiskContentStore.TryDiscoverAsync(fileArtifact, artifactExpandedPath, outputDirectoryRoot: outputDirectoryRoot);
await LocalDiskContentStore.TryDiscoverAsync(fileArtifact, artifactExpandedPath, outputDirectoryRoot: outputDirectoryRoot, isExecutable: isExecutable.Result);
DiscoveredContentHashOrigin origin;
if (possiblyDiscovered.Succeeded)

Просмотреть файл

@ -197,7 +197,8 @@ namespace BuildXL.Scheduler
{
FileMaterializationInfo sourceMaterializationInfo = environment.State.FileContentManager.GetInputContent(pip.Source);
FileContentInfo sourceContentInfo = sourceMaterializationInfo.FileContentInfo;
// Determine whether the destination file of the CopyFile pip is an executable or not.
bool isDestinationExecutable;
ReadOnlyArray<AbsolutePath> symlinkChain;
var symlinkTarget = AbsolutePath.Invalid;
bool isSymLink = false;
@ -250,6 +251,8 @@ namespace BuildXL.Scheduler
{
// We assume that source files cannot be made read-only so we use copy file materialization
// rather than hardlinking
// If the source of the CopyFile pip is a symlink then we check if the symlink target file is an executable or not.
isDestinationExecutable = FileUtilities.TryGetIsExecutableIfNeeded(symlinkTarget.ToString(pathTable)).Result;
var maybeStored = await environment.LocalDiskContentStore.TryStoreAsync(
environment.Cache.ArtifactContentCache,
fileRealizationModes: FileRealizationMode.Copy,
@ -262,7 +265,8 @@ namespace BuildXL.Scheduler
// Source should have been tracked by hash-source file pip or by CheckValidSymlinkChainAsync, no need to retrack.
trackPath: false,
// A copy file is never a dynamic output
outputDirectoryRoot: AbsolutePath.Invalid);
outputDirectoryRoot: AbsolutePath.Invalid,
isExecutable: isDestinationExecutable);
if (!maybeStored.Succeeded)
{
@ -278,14 +282,18 @@ namespace BuildXL.Scheduler
possiblyTracked.Failure.Throw();
}
}
else
{
// If the source of the copy file pip is not symlink, then we use the isExecutable property from sourceMaterializationInfo to determine whether its an executable or not.
isDestinationExecutable = sourceMaterializationInfo.IsExecutable;
}
// Just pass through the hash
environment.State.FileContentManager.ReportOutputContent(
operationContext,
pip.SemiStableHash,
pip.Destination,
// TODO: Should we maintain the case of the source file?
FileMaterializationInfo.CreateWithUnknownName(sourceContentInfo),
FileMaterializationInfo.CreateWithUnknownName(sourceContentInfo, isExecutable: isDestinationExecutable),
PipOutputOrigin.NotMaterialized);
var result = PipResultStatus.NotMaterialized;
@ -305,8 +313,7 @@ namespace BuildXL.Scheduler
operationContext,
pip.SemiStableHash,
pip.Destination,
// TODO: Should we maintain the case of the source file?
FileMaterializationInfo.CreateWithUnknownName(sourceContentInfo),
FileMaterializationInfo.CreateWithUnknownName(sourceContentInfo, isExecutable: isDestinationExecutable),
PipOutputOrigin.Produced);
var possiblyTracked = await TrackSymlinkChain(symlinkChain);
@ -4396,7 +4403,7 @@ namespace BuildXL.Scheduler
Contract.Requires(cachedArtifactContentHashes != null);
var allHashes =
new List<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot)>(
new List<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot, bool isExecutable)>(
cachedArtifactContentHashes.Count + (standardError == null ? 0 : 1) + (standardOutput == null ? 0 : 1));
// only check/load "real" files - reparse points are not stored in CAS, they are stored in metadata that we have already obtained
@ -4405,16 +4412,16 @@ namespace BuildXL.Scheduler
// Also, do not load zero-hash files (there is nothing in CAS with this hash)
allHashes.AddRange(cachedArtifactContentHashes
.Where(pair => pair.fileMaterializationInfo.IsCacheable)
.Select(a => (a.fileArtifact, a.fileMaterializationInfo.Hash, a.fileMaterializationInfo.OpaqueDirectoryRoot)));
.Select(a => (a.fileArtifact, a.fileMaterializationInfo.Hash, a.fileMaterializationInfo.OpaqueDirectoryRoot, a.fileMaterializationInfo.IsExecutable)));
if (standardOutput != null)
{
allHashes.Add((FileArtifact.CreateOutputFile(standardOutput.Item1), standardOutput.Item2, AbsolutePath.Invalid));
allHashes.Add((FileArtifact.CreateOutputFile(standardOutput.Item1), standardOutput.Item2, AbsolutePath.Invalid, false));
}
if (standardError != null)
{
allHashes.Add((FileArtifact.CreateOutputFile(standardError.Item1), standardError.Item2, AbsolutePath.Invalid));
allHashes.Add((FileArtifact.CreateOutputFile(standardError.Item1), standardError.Item2, AbsolutePath.Invalid, false));
}
// Check whether the cache provides a strong guarantee that content will be available for a successful pin

Просмотреть файл

@ -1,6 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.
using System;
using System.IO;
using BuildXL.Native.IO;
using BuildXL.Utilities.Core;
using Test.BuildXL.Executables.TestProcess;
@ -14,6 +16,8 @@ namespace IntegrationTest.BuildXL.Scheduler
[TestClassIfSupported(requiresUnixBasedOperatingSystem: true)]
public class LinuxPermissionTests : SchedulerIntegrationTestBase
{
private const string TextToCopied = "Hello world";
public LinuxPermissionTests(ITestOutputHelper output) : base(output)
{
}
@ -85,5 +89,114 @@ namespace IntegrationTest.BuildXL.Scheduler
XAssert.IsTrue(result.Succeeded && result.Result);
}
/// <summary>
/// Ensure that the destination file has the execute permission bit set if the source of the copy file pip is a source file.
/// This test covers the scenario where the 'storeOutputsToCache' flag is either enabled or disabled.
/// </summary>
[Theory]
[InlineData(true)]
[InlineData(false)]
public void SetExecutePermissionOnDestinationWhenCopyPipSourceIsASourceFile(bool storeOutputsToCache)
{
Configuration.Schedule.StoreOutputsToCache = storeOutputsToCache;
Configuration.Engine.UseHardlinks = false;
// Source file to copy from
var sourceFileForCopyPip = CreateSourceFile();
// Path to copy to
AbsolutePath destinationPath = CreateUniqueObjPath("destinationCopyFile");
_ = FileUtilities.TrySetExecutePermissionIfNeeded(sourceFileForCopyPip.Path.ToString(Context.PathTable));
// Create copy file pip and adds it to the graph
var copiedFileA = CopyFile(sourceFileForCopyPip, destinationPath);
// Run the scheduler and check the file permissions on the destination of the copy file.
RunSchedulerAndCheckFilePermissionsOnDestination(destinationPath);
}
/// <summary>
/// Ensure that the destination file has the execute permission bit set if the source of the copy file pip is an output of another pip.
/// </summary>
[Fact]
public void SetExecutePermissionOnDestinationWhenCopyPipSourceIsAnOutputOfAnotherPip()
{
// Source file to copy from
var sourceFileForCopyPip = CreateOutputFileArtifact();
// Path to copy to
AbsolutePath destinationPath = CreateUniqueObjPath("destinationCopyFile");
var ops = (new Operation[]
{
Operation.WriteFile(sourceFileForCopyPip, TextToCopied),
Operation.SetExecutionPermissions(sourceFileForCopyPip)
});
var pipBuilderA = CreatePipBuilder(ops);
var scheduledProcessA = SchedulePipBuilder(pipBuilderA);
// Create copy file pip and adds it to the graph
var copiedFileA = CopyFile(sourceFileForCopyPip, destinationPath);
// First run should be a cache miss, second one a cache hit.
RunScheduler().AssertCacheMiss(scheduledProcessA.Process.PipId);
// Check to ensure that the destination file also has the execute permission bit set.
XAssert.IsTrue(FileUtilities.TryGetIsExecutableIfNeeded(destinationPath.ToString(Context.PathTable)).Result,
"Execute permission not set on destination of copy file!");
// Delete the destination file and run the scheduler again.
Delete(sourceFileForCopyPip);
RunScheduler().AssertCacheHit(scheduledProcessA.Process.PipId);
// Content of source and destination should be same.
string actualContent = File.ReadAllText(copiedFileA.Path.ToString(Context.PathTable));
XAssert.AreEqual(TextToCopied, actualContent);
// Check to ensure that the destination file also has the execute permission bit set.
XAssert.IsTrue(FileUtilities.TryGetIsExecutableIfNeeded(destinationPath.ToString(Context.PathTable)).Result,
"Execute permission not set on destination of copy file!");
}
/// <summary>
/// Ensure that the destination file has the execute permission bit set if the symlink target has the bit set.
/// </summary>
[Theory]
[InlineData(true)]
[InlineData(false)]
public void SetExecutePermissionOnDestionationForSymlink(bool storeOutputsToCache)
{
Configuration.Schedule.AllowCopySymlink = true;
Configuration.Schedule.StoreOutputsToCache = storeOutputsToCache;
// Symlink chain:
// symlinkFile1 -> symlinkFile2 -> targetFile
FileArtifact targetFile = CreateSourceFile();
FileArtifact symlinkFile1 = CreateOutputFileArtifact();
FileArtifact symlinkFile2 = CreateOutputFileArtifact();
FileArtifact destination = CreateOutputFileArtifact();
// only the head of the symlink chain is created during the build -> valid chain
XAssert.PossiblySucceeded(FileUtilities.TryCreateSymbolicLink(ArtifactToString(symlinkFile2), ArtifactToString(targetFile), isTargetFile: true));
_ = FileUtilities.TrySetExecutePermissionIfNeeded(targetFile.Path.ToString(Context.PathTable));
CreateAndSchedulePipBuilder(new Operation[]
{
Operation.CreateSymlink(symlinkFile1, symlinkFile2, Operation.SymbolicLinkFlag.FILE),
});
// CopyFilePip : copy symlinkFile1 to output
FileArtifact output = CopyFile(symlinkFile1, destination);
RunSchedulerAndCheckFilePermissionsOnDestination(destination);
}
public void RunSchedulerAndCheckFilePermissionsOnDestination(AbsolutePath destination)
{
RunScheduler().AssertSuccess();
// Check to ensure that the destination file also has the execute permission bit set.
XAssert.IsTrue(FileUtilities.TryGetIsExecutableIfNeeded(destination.ToString(Context.PathTable)).Result,
"Execute permission not set on destination of copy file!");
// Delete the destination before running the build again.
Delete(destination);
RunScheduler().AssertSuccess();
// Check to ensure that the destination file also has the execute permission bit set.
XAssert.IsTrue(FileUtilities.TryGetIsExecutableIfNeeded(destination.ToString(Context.PathTable)).Result,
"Execute permission not set on destination of copy file!");
}
}
}

Просмотреть файл

@ -301,12 +301,13 @@ namespace Test.BuildXL.Scheduler
XAssert.IsTrue(possiblyStored.Succeeded);
harness.DeleteFileIfExists(outputFile.Path);
var filesAndContentHashes = new List<(FileArtifact, ContentHash, AbsolutePath)>
var filesAndContentHashes = new List<(FileArtifact, ContentHash, AbsolutePath, bool)>
{
(
outputFile,
possiblyStored.Result.Hash,
AbsolutePath.Invalid)
AbsolutePath.Invalid,
false)
};
var loadAvailableOutput = await harness.FileContentManager.TryLoadAvailableOutputContentAsync(
@ -354,12 +355,13 @@ namespace Test.BuildXL.Scheduler
harness.Environment.InMemoryContentCache.DiscardContentIfPresent(possiblyStored.Result.Hash, CacheSites.LocalAndRemote);
var filesAndContentHashes = new List<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot)>
var filesAndContentHashes = new List<(FileArtifact fileArtifact, ContentHash contentHash, AbsolutePath outputDirectoryRoot, bool isExecutable)>
{
(
outputFile,
possiblyStored.Result.Hash,
AbsolutePath.Invalid)
AbsolutePath.Invalid,
false)
};
var loadAvailableOutput = await harness.FileContentManager.TryLoadAvailableOutputContentAsync(

Просмотреть файл

@ -98,18 +98,18 @@ namespace BuildXL.Storage
/// Creates a <see cref="FileMaterializationInfo"/> with a hash but no file name and length.
/// This is intended for abstract hashes that don't correspond to real files on disk.
/// </summary>
public static FileMaterializationInfo CreateWithUnknownLength(ContentHash hash)
public static FileMaterializationInfo CreateWithUnknownLength(ContentHash hash, bool isExecutable = false)
{
return new FileMaterializationInfo(FileContentInfo.CreateWithUnknownLength(hash), PathAtom.Invalid, AbsolutePath.Invalid, RelativePath.Invalid);
return new FileMaterializationInfo(FileContentInfo.CreateWithUnknownLength(hash), PathAtom.Invalid, AbsolutePath.Invalid, RelativePath.Invalid, isExecutable: isExecutable);
}
/// <summary>
/// Creates a <see cref="FileMaterializationInfo"/> with a hash but no file name.
/// This is intended for abstract hashes that don't correspond to real files on disk.
/// </summary>
public static FileMaterializationInfo CreateWithUnknownName(in FileContentInfo contentInfo)
public static FileMaterializationInfo CreateWithUnknownName(in FileContentInfo contentInfo, bool isExecutable = false)
{
return new FileMaterializationInfo(contentInfo, PathAtom.Invalid, AbsolutePath.Invalid, RelativePath.Invalid);
return new FileMaterializationInfo(contentInfo, PathAtom.Invalid, AbsolutePath.Invalid, RelativePath.Invalid, isExecutable: isExecutable);
}
/// <summary>

2
bxl.sh
Просмотреть файл

@ -329,7 +329,7 @@ fi
# Forcing a salt here to avoid problems faced in Linux validation pipeline related to cache.
# This is related to Bug 2104538 where the cache may or may not be setting the execute bit for some executables.
if [[ $arg_UserProvidedBxlArguments != *"/p:BUILDXL_FINGERPRINT_SALT"* ]]; then
arg_Positional+=("/p:BUILDXL_FINGERPRINT_SALT=forceSaltForLinuxBugFixPR")
arg_Positional+=("/p:BUILDXL_FINGERPRINT_SALT=fixForCopyFilePipBugInLinux")
fi
compileWithBxl ${arg_Positional[@]} ${arg_UserProvidedBxlArguments[@]}