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

Don't ever claim /dev/null is an execpath. #12516

Closed
wants to merge 1 commit 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 @@ -13,7 +13,6 @@
// limitations under the License.
package com.google.devtools.build.lib.actions.cache;

import com.google.common.base.Preconditions;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.util.StreamWriter;
Expand All @@ -27,6 +26,11 @@
* OutputStream.
*/
public interface VirtualActionInput extends ActionInput, StreamWriter {
/**
* An empty virtual artifact <b>without</b> an execpath. This is used to denote empty files in
* runfiles and filesets.
*/
public static final VirtualActionInput EMPTY_MARKER = new EmptyActionInput();

/**
* Gets a {@link ByteString} representation of the fake file. Used to avoid copying if the fake
Expand All @@ -48,15 +52,7 @@ default FileArtifactValue getMetadata() throws IOException {
* use instances of this class to represent those files.
*/
final class EmptyActionInput implements VirtualActionInput {
private final PathFragment execPath;

public EmptyActionInput(PathFragment execPath) {
this.execPath = Preconditions.checkNotNull(execPath);
}

public EmptyActionInput(String execPath) {
this(PathFragment.create(execPath));
}
private EmptyActionInput() {}

@Override
public boolean isSymlink() {
Expand All @@ -65,12 +61,12 @@ public boolean isSymlink() {

@Override
public String getExecPathString() {
return execPath.getPathString();
throw new UnsupportedOperationException("empty virtual artifact doesn't have an execpath");
}

@Override
public PathFragment getExecPath() {
return execPath;
throw new UnsupportedOperationException("empty virtual artifact doesn't have an execpath");
}

@Override
Expand All @@ -85,7 +81,7 @@ public ByteString getBytes() throws IOException {

@Override
public String toString() {
return "EmptyActionInput: " + execPath;
return "EmptyActionInput";
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.vfs.Path;
Expand All @@ -48,8 +48,6 @@
* laid out.
*/
public class SpawnInputExpander {
public static final ActionInput EMPTY_FILE = new EmptyActionInput("/dev/null");

private final Path execRoot;
private final boolean strict;
private final RelativeSymlinkBehavior relSymlinkBehavior;
Expand Down Expand Up @@ -148,7 +146,7 @@ void addRunfilesToInputs(
addMapping(inputMap, location, localArtifact);
}
} else {
addMapping(inputMap, location, EMPTY_FILE);
addMapping(inputMap, location, VirtualActionInput.EMPTY_MARKER);
}
}
}
Expand Down Expand Up @@ -196,10 +194,10 @@ void addFilesetManifest(

for (Map.Entry<PathFragment, String> mapping : filesetManifest.getEntries().entrySet()) {
String value = mapping.getValue();
ActionInput artifact =
value == null
? EMPTY_FILE
: ActionInputHelper.fromPath(execRoot.getRelative(value).getPathString());
ActionInput artifact =
value == null
? VirtualActionInput.EMPTY_MARKER
: ActionInputHelper.fromPath(execRoot.getRelative(value).getPathString());
addMapping(inputMappings, mapping.getKey(), artifact);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.analysis.Runfiles;
import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl;
Expand Down Expand Up @@ -251,7 +252,7 @@ public void testRunfilesRootSymlink() throws Exception {
// directory gets created.
assertThat(inputMappings)
.containsEntry(
PathFragment.create("runfiles/workspace/.runfile"), SpawnInputExpander.EMPTY_FILE);
PathFragment.create("runfiles/workspace/.runfile"), VirtualActionInput.EMPTY_MARKER);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.clock.JavaClock;
import com.google.devtools.build.lib.exec.SpawnInputExpander;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.InMemoryCacheClient;
Expand Down Expand Up @@ -141,7 +140,7 @@ public void testStagingEmptyVirtualActionInput() throws Exception {

// act
actionInputFetcher.prefetchFiles(
ImmutableList.of(SpawnInputExpander.EMPTY_FILE), metadataProvider);
ImmutableList.of(VirtualActionInput.EMPTY_MARKER), metadataProvider);

// assert that nothing happened
assertThat(actionInputFetcher.downloadedFiles()).isEmpty();
Expand Down