Skip to content

Commit

Permalink
Remove some deadish Windows-related runfiles code.
Browse files Browse the repository at this point in the history
In particular, SymlinkTreeAction no longer needs to accept artifacts
as an input. --experimental_enable_runfiles now immediately reports an
error on Windows.

This mostly unwinds e4974e4
("Separate runfiles middlemen into two layers") and
41f4456 ("Make runfiles tree creation
on Windows depend on the artifacts of the actual runfiles."). See
https://groups.google.com/d/msg/bazel-dev/btOAgxv434g/bDhTOOePAgAJ.

Change-Id: Iac3308669bfc07abfd1c91445922269d8fdc2a26
PiperOrigin-RevId: 177837504
  • Loading branch information
benjaminp authored and Copybara-Service committed Dec 4, 2017
1 parent 2877735 commit 0885abd
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ private RunfilesSupport(
Artifact artifactsMiddleman = createArtifactsMiddleman(ruleContext, runfiles.getAllArtifacts());
if (createManifest) {
runfilesInputManifest = createRunfilesInputManifestArtifact(ruleContext);
runfilesManifest = createRunfilesAction(ruleContext, runfiles, artifactsMiddleman);
runfilesManifest = createRunfilesAction(ruleContext, runfiles);
} else {
runfilesInputManifest = runfilesManifest =
createManifestMiddleman(ruleContext, runfiles, artifactsMiddleman);
Expand Down Expand Up @@ -282,16 +282,14 @@ private Artifact createRunfilesMiddleman(ActionConstructionContext context,
}

/**
* Creates a runfiles action for all of the specified files, and returns the
* output artifact (the artifact for the MANIFEST file).
* Creates a runfiles action for all of the specified files, and returns the output artifact (the
* artifact for the MANIFEST file).
*
* <p>The "runfiles" action creates a symlink farm that links all the runfiles
* (which may come from different places, e.g. different package paths,
* generated files, etc.) into a single tree, so that programs can access them
* using the workspace-relative name.
* <p>The "runfiles" action creates a symlink farm that links all the runfiles (which may come
* from different places, e.g. different package paths, generated files, etc.) into a single tree,
* so that programs can access them using the workspace-relative name.
*/
private Artifact createRunfilesAction(ActionConstructionContext context, Runfiles runfiles,
Artifact artifactsMiddleman) {
private Artifact createRunfilesAction(ActionConstructionContext context, Runfiles runfiles) {
// Compute the names of the runfiles directory and its MANIFEST file.
Artifact inputManifest = getRunfilesInputManifest();
context.getAnalysisEnvironment().registerAction(
Expand All @@ -316,7 +314,6 @@ private Artifact createRunfilesAction(ActionConstructionContext context, Runfile
new SymlinkTreeAction(
context.getActionOwner(),
inputManifest,
artifactsMiddleman,
outputManifest,
/*filesetTree=*/ false,
config.getLocalShellEnvironment(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.lib.vfs.Path;
import javax.annotation.Nullable;

/**
* Action responsible for the symlink tree creation.
Expand All @@ -47,9 +45,6 @@ public final class SymlinkTreeAction extends AbstractAction {
* Creates SymlinkTreeAction instance.
* @param owner action owner
* @param inputManifest the input runfiles manifest
* @param artifactMiddleman the middleman artifact representing all the files the symlinks
* point to (on Windows we need to know if the target of a "symlink" is
* a directory or a file so we need to build it before)
* @param outputManifest the generated symlink tree manifest
* (must have "MANIFEST" base name). Symlink tree root
* will be set to the artifact's parent directory.
Expand All @@ -59,12 +54,11 @@ public final class SymlinkTreeAction extends AbstractAction {
public SymlinkTreeAction(
ActionOwner owner,
Artifact inputManifest,
@Nullable Artifact artifactMiddleman,
Artifact outputManifest,
boolean filesetTree,
ImmutableMap<String, String> shellEnvironment,
boolean enableRunfiles) {
super(owner, computeInputs(inputManifest, artifactMiddleman), ImmutableList.of(outputManifest));
super(owner, ImmutableList.of(inputManifest), ImmutableList.of(outputManifest));
Preconditions.checkArgument(outputManifest.getPath().getBaseName().equals("MANIFEST"));
this.inputManifest = inputManifest;
this.outputManifest = outputManifest;
Expand All @@ -73,19 +67,6 @@ public SymlinkTreeAction(
this.enableRunfiles = enableRunfiles;
}

private static ImmutableList<Artifact> computeInputs(
Artifact inputManifest, Artifact artifactMiddleman) {
ImmutableList.Builder<Artifact> result = ImmutableList.<Artifact>builder()
.add(inputManifest);
if (artifactMiddleman != null) {
Path path = artifactMiddleman.getPath();
if (!path.getFileSystem().supportsSymbolicLinksNatively(path)) {
result.add(artifactMiddleman);
}
}
return result.build();
}

public Artifact getInputManifest() {
return inputManifest;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1274,6 +1274,10 @@ public void reportInvalidOptions(EventHandler reporter) {
fragment.reportInvalidOptions(reporter, this.buildOptions);
}

if (OS.getCurrent() == OS.WINDOWS && runfilesEnabled()) {
reporter.handle(Event.error("building runfiles is not supported on Windows"));
}

if (options.outputDirectoryName != null) {
reporter.handle(Event.error(
"The internal '--output directory name' option cannot be used on the command line"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,11 @@ public Pair<Artifact, Runfiles> createApkBuilderSymlinks(RuleContext ruleContext
.build();
ruleContext.registerAction(sourceManifestAction);
Artifact outputManifest = AndroidBinary.getDxArtifact(ruleContext, "native_symlinks/MANIFEST");
Artifact nativeLibsMiddleman =
ruleContext.getAnalysisEnvironment().getMiddlemanFactory().createRunfilesMiddleman(
ruleContext.getActionOwner(), null, symlinks.values(),
ruleContext.getConfiguration().getMiddlemanDirectory(
ruleContext.getRule().getRepository()), "android_native_libs");

ruleContext.registerAction(
new SymlinkTreeAction(
ruleContext.getActionOwner(),
inputManifest,
nativeLibsMiddleman,
outputManifest,
false,
ruleContext.getConfiguration().getLocalShellEnvironment(),
Expand Down
6 changes: 6 additions & 0 deletions src/test/py/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ py_test(
deps = [":test_base"],
)

py_test(
name = "runfiles_test",
srcs = ["runfiles_test.py"],
deps = [":test_base"],
)

py_test(
name = "bazel_windows_cpp_test",
size = "large",
Expand Down
34 changes: 34 additions & 0 deletions src/test/py/bazel/runfiles_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# pylint: disable=g-bad-file-header
# Copyright 2017 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.

import unittest
from src.test.py.bazel import test_base


class RunfilesTest(test_base.TestBase):

def testAttemptToBuildRunfilesOnWindows(self):
if not self.IsWindows():
self.skipTest("only applicable to Windows")
self.ScratchFile("WORKSPACE")
exit_code, _, stderr = self.RunBazel(
["build", "--experimental_enable_runfiles"])
self.assertNotEqual(exit_code, 0)
self.assertIn("building runfiles is not supported on Windows",
"\n".join(stderr))


if __name__ == "__main__":
unittest.main()

0 comments on commit 0885abd

Please sign in to comment.