From 8818a57fce32a7872f045f03a334e1c9403724d3 Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Tue, 29 Nov 2022 15:33:00 -0500 Subject: [PATCH] =?UTF-8?q?[6.0.0]=20Only=20inject=20metadata=20for=20outp?= =?UTF-8?q?uts=20that=20cannot=20be=20reconstructed=20by=20skyf=E2=80=A6?= =?UTF-8?q?=20(#16879)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update RemoteActionFileSystem to apply permission changes to local files even if remote file exists. Previously, it only applies permission changes to local files when the remote one doesn't exist. It is fine because the call sites only use these method when remote file are missing. However, this isn't true with future changes. PiperOrigin-RevId: 490872822 Change-Id: I7a19d99cd828294cbafa7b5f3fdc368d64e556ec * Fix permission operations for the case of only remote directory. PiperOrigin-RevId: 491280334 Change-Id: I30afef9f069eca8aee4d983664f42b3961e95adf * Only inject metadata for outputs that cannot be reconstructed by skyframe later Currently, they are symlinks. For other outputs, let skyframe read action file system to construct the metadata. Before this change, we inject metadata of symlink outputs, tree outputs and file outputs inside `RemoteActionFileSystem#flush()` if these outputs are stored inside the in-memory fs. If the outputs are somehow generated in the local fs, skyframe will read the fs later to construct metadata for them. However, for tree outputs, skyframe always create an empty directory before action execution in the in-memory fs. So inside the `flush`, we always inject metadata for it. It means local tree outputs are ignored. We could update the code to also read local file system when reading tree files, but then the problem is how to construct metadata from file status which is well done by skyframe. So instead of injecting metadata by traversal the filesystem inside `flush`, we just let skyframe to do the job. Fixes #16789. Closes #16812. PiperOrigin-RevId: 491622005 Change-Id: I10434e6856a1b2a207f39e07122a9b646edf518c Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com> --- .../google/devtools/build/lib/actions/BUILD | 2 + .../build/lib/actions/RemoteFileStatus.java | 28 + .../lib/remote/RemoteActionFileSystem.java | 124 ++--- .../lib/skyframe/ActionMetadataHandler.java | 15 +- ...ildWithoutTheBytesIntegrationTestBase.java | 37 ++ .../RemoteActionFileSystemTestBase.java | 498 ++++++++++++++++++ 6 files changed, 640 insertions(+), 64 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 925dfcc790291a..4c473b50961b02 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -57,6 +57,7 @@ java_library( "ResourceManager.java", "ResourceSet.java", "ResourceSetOrBuilder.java", + "RemoteFileStatus.java", "PackageRootResolver.java", "PackageRoots.java", "ThreadStateReceiver.java", @@ -263,6 +264,7 @@ java_library( "FileStateType.java", "FileStateValue.java", "FileValue.java", + "RemoteFileStatus.java", "cache/MetadataDigestUtils.java", ], deps = [ diff --git a/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java b/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java new file mode 100644 index 00000000000000..73184b55e7cc30 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java @@ -0,0 +1,28 @@ +// Copyright 2022 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.actions; + +import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.vfs.FileStatusWithDigest; + +/** + * A FileStatus that exists remotely and provides remote metadata. + * + *

Filesystem may return implementation of this interface if the files are stored remotely. When + * checking outputs of actions, Skyframe will inject the metadata returned by {@link + * #getRemoteMetadata()} if the output file has {@link RemoteFileStatus}. + */ +public interface RemoteFileStatus extends FileStatusWithDigest { + RemoteFileArtifactValue getRemoteMetadata(); +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index b7f31149f9f46f..7916bb802322ef 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.actions.cache.MetadataInjector; import com.google.devtools.build.lib.clock.Clock; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; @@ -128,41 +129,7 @@ void flush() throws IOException { PathFragment path = execRoot.getRelative(entry.getKey()); Artifact output = entry.getValue(); - if (maybeInjectMetadataForSymlink(path, output)) { - continue; - } - - if (output.isTreeArtifact()) { - if (remoteOutputTree.exists(path)) { - SpecialArtifact parent = (SpecialArtifact) output; - TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent); - - // TODO: Check directory content on the local fs to support mixed tree. - TreeArtifactValue.visitTree( - remoteOutputTree.getPath(path), - (parentRelativePath, type) -> { - if (type == Dirent.Type.DIRECTORY) { - return; - } - RemoteFileInfo remoteFile = - remoteOutputTree.getRemoteFileInfo( - path.getRelative(parentRelativePath), /* followSymlinks= */ true); - if (remoteFile != null) { - TreeFileArtifact child = - TreeFileArtifact.createTreeOutput(parent, parentRelativePath); - tree.putChild(child, createRemoteMetadata(remoteFile)); - } - }); - - metadataInjector.injectTree(parent, tree.build()); - } - } else { - RemoteFileInfo remoteFile = - remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ true); - if (remoteFile != null) { - metadataInjector.injectFile(output, createRemoteMetadata(remoteFile)); - } - } + maybeInjectMetadataForSymlink(path, output); } } @@ -183,26 +150,24 @@ void flush() throws IOException { * the output should be materialized as a symlink to the original location, which avoids * fetching multiple copies when multiple symlinks to the same artifact are created in the * same build. - * - * @return Whether the metadata was injected. */ - private boolean maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact output) + private void maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact output) throws IOException { if (output.isSymlink()) { - return false; + return; } Path outputTreePath = remoteOutputTree.getPath(linkPath); if (!outputTreePath.exists(Symlinks.NOFOLLOW)) { - return false; + return; } PathFragment targetPath; try { targetPath = outputTreePath.readSymbolicLink(); } catch (NotASymlinkException e) { - return false; + return; } checkState( @@ -212,7 +177,7 @@ private boolean maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact ou if (output.isTreeArtifact()) { TreeArtifactValue metadata = getRemoteTreeMetadata(targetPath); if (metadata == null) { - return false; + return; } SpecialArtifact parent = (SpecialArtifact) output; @@ -233,7 +198,7 @@ private boolean maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact ou } else { RemoteFileArtifactValue metadata = getRemoteMetadata(targetPath); if (metadata == null) { - return false; + return; } RemoteFileArtifactValue injectedMetadata = @@ -247,8 +212,6 @@ private boolean maybeInjectMetadataForSymlink(PathFragment linkPath, Artifact ou metadataInjector.injectFile(output, injectedMetadata); } - - return true; } private RemoteFileArtifactValue createRemoteMetadata(RemoteFileInfo remoteFile) { @@ -313,54 +276,85 @@ protected byte[] getDigest(PathFragment path) throws IOException { } // -------------------- File Permissions -------------------- + // Remote files are always readable, writable and executable since we can't control their + // permissions. + + private boolean existsInMemory(PathFragment path) { + return remoteOutputTree.getPath(path).isDirectory() || getRemoteMetadata(path) != null; + } @Override protected boolean isReadable(PathFragment path) throws IOException { - FileArtifactValue m = getRemoteMetadata(path); - return m != null || super.isReadable(path); + return existsInMemory(path) || super.isReadable(path); } @Override protected boolean isWritable(PathFragment path) throws IOException { - FileArtifactValue m = getRemoteMetadata(path); - return m != null || super.isWritable(path); + if (existsInMemory(path)) { + // If path exists locally, also check whether it's writable. We need this check for the case + // where the action need to delete their local outputs but the parent directory is not + // writable. + try { + return super.isWritable(path); + } catch (FileNotFoundException e) { + // Intentionally ignored + return true; + } + } + + return super.isWritable(path); } @Override protected boolean isExecutable(PathFragment path) throws IOException { - FileArtifactValue m = getRemoteMetadata(path); - return m != null || super.isExecutable(path); + return existsInMemory(path) || super.isExecutable(path); } @Override protected void setReadable(PathFragment path, boolean readable) throws IOException { - RemoteFileArtifactValue m = getRemoteMetadata(path); - if (m == null) { + try { super.setReadable(path, readable); + } catch (FileNotFoundException e) { + // in case of missing in-memory path, re-throw the error. + if (!existsInMemory(path)) { + throw e; + } } } @Override public void setWritable(PathFragment path, boolean writable) throws IOException { - RemoteFileArtifactValue m = getRemoteMetadata(path); - if (m == null) { + try { super.setWritable(path, writable); + } catch (FileNotFoundException e) { + // in case of missing in-memory path, re-throw the error. + if (!existsInMemory(path)) { + throw e; + } } } @Override protected void setExecutable(PathFragment path, boolean executable) throws IOException { - RemoteFileArtifactValue m = getRemoteMetadata(path); - if (m == null) { + try { super.setExecutable(path, executable); + } catch (FileNotFoundException e) { + // in case of missing in-memory path, re-throw the error. + if (!existsInMemory(path)) { + throw e; + } } } @Override protected void chmod(PathFragment path, int mode) throws IOException { - RemoteFileArtifactValue m = getRemoteMetadata(path); - if (m == null) { + try { super.chmod(path, mode); + } catch (FileNotFoundException e) { + // in case of missing in-memory path, re-throw the error. + if (!existsInMemory(path)) { + throw e; + } } } @@ -467,7 +461,12 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx } private static FileStatus statFromRemoteMetadata(RemoteFileArtifactValue m) { - return new FileStatus() { + return new RemoteFileStatus() { + @Override + public byte[] getDigest() { + return m.getDigest(); + } + @Override public boolean isFile() { return m.getType().isFile(); @@ -507,6 +506,11 @@ public long getLastChangeTime() { public long getNodeId() { throw new UnsupportedOperationException("Cannot get node id for " + m); } + + @Override + public RemoteFileArtifactValue getRemoteMetadata() { + return m; + } }; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index 77104d996b1ee3..f6e45f06189dcb 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.actions.FilesetManifest; import com.google.devtools.build.lib.actions.FilesetManifest.RelativeSymlinkBehaviorWithoutError; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; +import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.actions.cache.MetadataHandler; import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor; import com.google.devtools.build.lib.vfs.DigestUtils; @@ -656,14 +657,20 @@ private static FileArtifactValue fileArtifactValueFromStat( return FileArtifactValue.MISSING_FILE_MARKER; } + if (stat.isDirectory()) { + return FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime()); + } + + if (stat instanceof RemoteFileStatus) { + return ((RemoteFileStatus) stat).getRemoteMetadata(); + } + FileStateValue fileStateValue = FileStateValue.createWithStatNoFollow( rootedPath, stat, digestWillBeInjected, xattrProvider, tsgm); - return stat.isDirectory() - ? FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime()) - : FileArtifactValue.createForNormalFile( - fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize()); + return FileArtifactValue.createForNormalFile( + fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize()); } private static void setPathReadOnlyAndExecutableIfFile(Path path) throws IOException { diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java index cf1dc3ffac39e6..54e214e3c0f793 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java @@ -378,6 +378,42 @@ public void downloadToplevel_treeArtifacts() throws Exception { assertValidOutputFile("foo/file-3", "file-3\n"); } + @Test + public void treeOutputsFromLocalFileSystem_works() throws Exception { + // Disable on Windows since it fails for unknown reasons. + // TODO(chiwang): Enable it on windows. + if (OS.getCurrent() == OS.WINDOWS) { + return; + } + + // Test that tree artifact generated locally can be consumed by other actions. + // See https://github.com/bazelbuild/bazel/issues/16789 + + // Disable remote execution so tree outputs are generated locally + addOptions("--modify_execution_info=OutputDir=+no-remote-exec"); + setDownloadToplevel(); + writeOutputDirRule(); + write( + "BUILD", + "load(':output_dir.bzl', 'output_dir')", + "output_dir(", + " name = 'foo',", + " manifest = ':manifest',", + ")", + "genrule(", + " name = 'foobar',", + " srcs = [':foo'],", + " outs = ['out/foobar.txt'],", + " cmd = 'cat $(location :foo)/file-1 > $@ && echo bar >> $@',", + ")"); + write("manifest", "file-1"); + + buildTarget("//:foobar"); + waitDownloads(); + + assertValidOutputFile("out/foobar.txt", "file-1\nbar\n"); + } + @Test public void incrementalBuild_deleteOutputsInUnwritableParentDirectory() throws Exception { write( @@ -513,6 +549,7 @@ protected void writeOutputDirRule() throws IOException { "def _output_dir_impl(ctx):", " output_dir = ctx.actions.declare_directory(ctx.attr.name)", " ctx.actions.run_shell(", + " mnemonic = 'OutputDir',", " inputs = [ctx.file.manifest],", " outputs = [output_dir],", " arguments = [ctx.file.manifest.path, output_dir.path],", diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java index a39e326175062b..02317f5d23eea0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTestBase.java @@ -328,4 +328,502 @@ public void getDirectoryEntries_nothingThere_throwsFileNotFound() throws IOExcep path -> path.getDirectoryEntries().stream().map(Path::getBaseName).collect(toImmutableList())); } + + @Test + public void isReadable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).isReadable()); + } + + @Test + public void isReadable_onlyRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isTrue(); + } + + @Test + public void isReadable_onlyRemoteDirectory_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isTrue(); + } + + @Test + public void isReadable_localReadableFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isTrue(); + } + + @Test + public void isReadable_localNonReadableFile_returnsFalse() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setReadable(false); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isFalse(); + } + + @Test + public void isReadable_localReadableFileAndRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isTrue(); + } + + @Test + public void isReadable_localNonReadableFileAndRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setReadable(false); + + var readable = actionFs.getPath(path).isReadable(); + + assertThat(readable).isTrue(); + } + + @Test + public void isWritable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).isWritable()); + } + + @Test + public void isWritable_onlyRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isTrue(); + } + + @Test + public void isWritable_onlyRemoteDirectory_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isTrue(); + } + + @Test + public void isWritable_localWritableFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isTrue(); + } + + @Test + public void isWritable_localNonWritableFile_returnsFalse() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setWritable(false); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isFalse(); + } + + @Test + public void isWritable_localWritableFileAndRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isTrue(); + } + + @Test + public void isWritable_localNonWritableFileAndRemoteFile_returnsFalse() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setWritable(false); + + var writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isFalse(); + } + + @Test + public void isWritable_localNonWritableDirectoryAndRemoteDirectory_returnsFalse() + throws Exception { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).getPath(path).createDirectoryAndParents(); + getLocalFileSystem(actionFs).getPath(path).createDirectoryAndParents(); + getLocalFileSystem(actionFs).getPath(path).setWritable(false); + + boolean writable = actionFs.getPath(path).isWritable(); + + assertThat(writable).isFalse(); + } + + @Test + public void isExecutable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).isExecutable()); + } + + @Test + public void isExecutable_onlyRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isTrue(); + } + + @Test + public void isExecutable_onlyRemoteDirecotry_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isTrue(); + } + + @Test + public void isExecutable_localExecutableFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setExecutable(true); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isTrue(); + } + + @Test + public void isExecutable_localNonExecutableFile_returnsFalse() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isFalse(); + } + + @Test + public void isExecutable_localExecutableFileAndRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + getLocalFileSystem(actionFs).getPath(path).setExecutable(true); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isTrue(); + } + + @Test + public void isExecutable_localNonExecutableFileAndRemoteFile_returnsTrue() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + + var executable = actionFs.getPath(path).isExecutable(); + + assertThat(executable).isTrue(); + } + + @Test + public void setReadable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).setReadable(false)); + } + + @Test + public void setReadable_onlyRemoteFile_remainsReadable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + actionFs.getPath(path).setReadable(false); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + } + + @Test + public void setReadable_onlyRemoteDirecotry_remainsReadable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + actionFs.getPath(path).setReadable(false); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + } + + @Test + public void setReadable_localFile_change() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + + actionFs.getPath(path).setReadable(false); + + assertThat(actionFs.getPath(path).isReadable()).isFalse(); + } + + @Test + public void setReadable_localFileAndRemoteFile_changeLocal() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isReadable()).isTrue(); + + actionFs.getPath(path).setReadable(false); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isReadable()).isFalse(); + } + + @Test + public void setWritable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).setWritable(false)); + } + + @Test + public void setWritable_onlyRemoteFile_remainsWritable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + actionFs.getPath(path).setWritable(false); + + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + } + + @Test + public void setWritable_onlyRemoteDirecotry_remainsWritable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + actionFs.getPath(path).setWritable(false); + + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + } + + @Test + public void setWritable_localFile_change() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + + actionFs.getPath(path).setWritable(false); + + assertThat(actionFs.getPath(path).isWritable()).isFalse(); + } + + @Test + public void setWritable_localFileAndRemoteFile_changeLocal() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isWritable()).isTrue(); + + actionFs.getPath(path).setWritable(false); + + assertThat(actionFs.getPath(path).isWritable()).isFalse(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isWritable()).isFalse(); + } + + @Test + public void setExecutable_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).setExecutable(false)); + } + + @Test + public void setExecutable_onlyRemoteFile_remainsExecutable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + + actionFs.getPath(path).setExecutable(false); + + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void setExecutable_onlyRemoteDirecotry_remainsExecutable() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + + actionFs.getPath(path).setExecutable(false); + + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void setExecutable_localFile_change() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isExecutable()).isFalse(); + + actionFs.getPath(path).setExecutable(true); + + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void setExecutable_localFileAndRemoteFile_changeLocal() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isExecutable()).isFalse(); + + actionFs.getPath(path).setExecutable(true); + + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isExecutable()).isTrue(); + } + + @Test + public void chmod_fileDoesNotExist_throwError() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + + assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).chmod(000)); + } + + @Test + public void chmod_onlyRemoteFile_remainsSame() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + + actionFs.getPath(path).chmod(000); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void chmod_onlyRemoteDirectory_remainsSame() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("dir"); + getRemoteFileSystem(actionFs).createDirectoryAndParents(path); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + + actionFs.getPath(path).chmod(000); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void chmod_localFile_change() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isFalse(); + + actionFs.getPath(path).chmod(0111); + + assertThat(actionFs.getPath(path).isReadable()).isFalse(); + assertThat(actionFs.getPath(path).isWritable()).isFalse(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + } + + @Test + public void chmod_localFileAndRemoteFile_changeLocal() throws IOException { + var actionFs = createActionFileSystem(); + var path = getOutputPath("file"); + injectRemoteFile(actionFs, path, "remote-content"); + writeLocalFile(actionFs, path, "local-content"); + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isTrue(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isReadable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isWritable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isExecutable()).isFalse(); + + actionFs.getPath(path).chmod(0111); + + assertThat(actionFs.getPath(path).isReadable()).isTrue(); + assertThat(actionFs.getPath(path).isWritable()).isFalse(); + assertThat(actionFs.getPath(path).isExecutable()).isTrue(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isReadable()).isFalse(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isWritable()).isFalse(); + assertThat(getLocalFileSystem(actionFs).getPath(path).isExecutable()).isTrue(); + } }