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

Fix runfiles creation with MANIFEST when building without the bytes #16972

Closed
wants to merge 4 commits into from
Closed
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
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,26 @@ protected ReadableByteChannel createReadableByteChannel(PathFragment path) throw

@Override
public void setLastModifiedTime(PathFragment path, long newTime) throws IOException {
RemoteFileArtifactValue m = getRemoteMetadata(path);
if (m == null) {
FileNotFoundException remoteException = null;
try {
remoteOutputTree.setLastModifiedTime(path, newTime);
} catch (FileNotFoundException e) {
remoteException = e;
}

FileNotFoundException localException = null;
try {
super.setLastModifiedTime(path, newTime);
} catch (FileNotFoundException e) {
localException = e;
}
remoteOutputTree.setLastModifiedTime(path, newTime);

if (remoteException == null || localException == null) {
return;
}

localException.addSuppressed(remoteException);
throw localException;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,32 @@ public void outputsAreNotDownloaded() throws Exception {
assertOutputsDoNotExist("//:foobar");
}

@Test
public void disableRunfiles_buildSuccessfully() throws Exception {
// Disable on Windows since it fails for unknown reasons.
// TODO(chiwang): Enable it on windows.
if (OS.getCurrent() == OS.WINDOWS) {
return;
}
write("BUILD",
"genrule(",
" name = 'foo',",
" cmd = 'echo foo > $@',",
" outs = ['foo.data'],",
")",
"sh_test(",
" name = 'foobar',",
" srcs = ['test.sh'],",
" data = [':foo'],",
")"
);
write("test.sh");
getWorkspace().getRelative("test.sh").setExecutable(true);
addOptions("--build_runfile_links", "--enable_runfiles=no");

buildTarget("//:foobar");
}

@Test
public void downloadOutputsWithRegex() throws Exception {
write(
Expand Down Expand Up @@ -108,7 +134,6 @@ public void downloadOutputsWithRegex_treeOutput_regexMatchesTreeFile() throws Ex
if (OS.getCurrent() == OS.WINDOWS) {
return;
}

writeOutputDirRule();
write(
"BUILD",
Expand Down Expand Up @@ -331,7 +356,6 @@ public void dynamicExecution_stdoutIsReported() throws Exception {
if (OS.getCurrent() == OS.WINDOWS) {
return;
}

addOptions("--internal_spawn_scheduler");
addOptions("--strategy=Genrule=dynamic");
addOptions("--experimental_local_execution_delay=9999999");
Expand Down Expand Up @@ -527,7 +551,6 @@ public void treeOutputsFromLocalFileSystem_works() throws Exception {
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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -826,4 +826,46 @@ public void chmod_localFileAndRemoteFile_changeLocal() throws IOException {
assertThat(getLocalFileSystem(actionFs).getPath(path).isWritable()).isFalse();
assertThat(getLocalFileSystem(actionFs).getPath(path).isExecutable()).isTrue();
}

@Test
public void setLastModifiedTime_fileDoesNotExist_throwError() throws IOException {
var actionFs = createActionFileSystem();
var path = getOutputPath("file");

assertThrows(FileNotFoundException.class, () -> actionFs.getPath(path).setLastModifiedTime(0));
}

@Test
public void setLastModifiedTime_onlyRemoteFile_doesntThrowError() throws IOException {
var actionFs = createActionFileSystem();
var path = getOutputPath("file");
injectRemoteFile(actionFs, path, "remote-content");

actionFs.getPath(path).setLastModifiedTime(0);
}

@Test
public void setLastModifiedTime_onlyLocalFile_successfullySet() throws IOException {
var actionFs = createActionFileSystem();
var path = getOutputPath("file");
writeLocalFile(actionFs, path, "local-content");
assertThat(actionFs.getPath(path).getLastModifiedTime()).isNotEqualTo(0);

actionFs.getPath(path).setLastModifiedTime(0);

assertThat(actionFs.getPath(path).getLastModifiedTime()).isEqualTo(0);
}

@Test
public void setLastModifiedTime_localAndRemoteFile_changeLocal() throws IOException {
var actionFs = createActionFileSystem();
var path = getOutputPath("file");
injectRemoteFile(actionFs, path, "remote-content");
writeLocalFile(actionFs, path, "local-content");
assertThat(getLocalFileSystem(actionFs).getPath(path).getLastModifiedTime()).isNotEqualTo(0);

actionFs.getPath(path).setLastModifiedTime(0);

assertThat(getLocalFileSystem(actionFs).getPath(path).getLastModifiedTime()).isEqualTo(0);
}
}