Get rid of a few BashSteps.
Summary:
Doing my bit to get rid of `BashStep`s from the code base :)
Check if source directory exists, and if it does, invoke `MkDirStep` and
`CopyStep`.
Test Plan:
unit tests.
diff --git a/src/com/facebook/buck/android/AndroidBinaryRule.java b/src/com/facebook/buck/android/AndroidBinaryRule.java
index 67596be..ac2d492 100644
--- a/src/com/facebook/buck/android/AndroidBinaryRule.java
+++ b/src/com/facebook/buck/android/AndroidBinaryRule.java
@@ -45,7 +45,6 @@
import com.facebook.buck.rules.RuleKey;
import com.facebook.buck.rules.SourcePath;
import com.facebook.buck.shell.AbstractGenruleStep;
-import com.facebook.buck.shell.BashStep;
import com.facebook.buck.shell.EchoStep;
import com.facebook.buck.shell.SymlinkFilesIntoDirectoryStep;
import com.facebook.buck.step.ExecutionContext;
@@ -64,6 +63,7 @@
import com.facebook.buck.zip.ZipDirectoryWithMaxDeflateStep;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
+import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.FluentIterable;
@@ -323,22 +323,51 @@
}
- // TODO(user): Replace the BashSteps in this class with Steps that are platform-agnostic.
- // You'll need to do this as part of making it possible to build an APK on Windows using Buck.
-
@VisibleForTesting
void copyNativeLibrary(String sourceDir,
String destinationDir,
ImmutableList.Builder<Step> commands) {
+ Path sourceDirPath = Paths.get(sourceDir);
+ Path destinationDirPath = Paths.get(destinationDir);
+
if (getCpuFilters().isEmpty()) {
- commands.add(new BashStep(String.format("cp -R %s/* %s", sourceDir, destinationDir)));
+ commands.add(new CopyStep(sourceDirPath, destinationDirPath, true));
} else {
for (TargetCpuType cpuType: getCpuFilters()) {
Optional<String> abiDirectoryComponent = getAbiDirectoryComponent(cpuType);
Preconditions.checkState(abiDirectoryComponent.isPresent());
- String libsDirectory = sourceDir + "/" + abiDirectoryComponent.get();
- commands.add(new BashStep(String.format(
- "[ -d %s ] && cp -R %s %s || exit 0", libsDirectory, libsDirectory, destinationDir)));
+
+ final Path libSourceDir = sourceDirPath.resolve(abiDirectoryComponent.get());
+ Path libDestinationDir = destinationDirPath.resolve(abiDirectoryComponent.get());
+
+ final MkdirStep mkDirStep = new MkdirStep(libDestinationDir);
+ final CopyStep copyStep = new CopyStep(libSourceDir, libDestinationDir, true);
+ commands.add(new Step() {
+ @Override
+ public int execute(ExecutionContext context) {
+ if (!context.getProjectFilesystem().exists(libSourceDir.toString())) {
+ return 0;
+ }
+ if (mkDirStep.execute(context) == 0 && copyStep.execute(context) == 0) {
+ return 0;
+ }
+ return 1;
+ }
+
+ @Override
+ public String getShortName() {
+ return "copy_native_libraries";
+ }
+
+ @Override
+ public String getDescription(ExecutionContext context) {
+ ImmutableList.Builder<String> stringBuilder = ImmutableList.builder();
+ stringBuilder.add(String.format("[ -d %s ]", libSourceDir.toString()));
+ stringBuilder.add(mkDirStep.getDescription(context));
+ stringBuilder.add(copyStep.getDescription(context));
+ return Joiner.on(" && ").join(stringBuilder.build());
+ }
+ });
}
}
}
diff --git a/test/com/facebook/buck/android/AndroidBinaryRuleTest.java b/test/com/facebook/buck/android/AndroidBinaryRuleTest.java
index b8d8ecc..4efa178 100644
--- a/test/com/facebook/buck/android/AndroidBinaryRuleTest.java
+++ b/test/com/facebook/buck/android/AndroidBinaryRuleTest.java
@@ -19,6 +19,7 @@
import static com.facebook.buck.util.BuckConstant.BIN_DIR;
import static com.facebook.buck.util.BuckConstant.GEN_DIR;
import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.replay;
import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
@@ -38,7 +39,6 @@
import com.facebook.buck.rules.FakeAbstractBuildRuleBuilderParams;
import com.facebook.buck.rules.FileSourcePath;
import com.facebook.buck.rules.SourcePath;
-import com.facebook.buck.shell.BashStep;
import com.facebook.buck.step.ExecutionContext;
import com.facebook.buck.step.Step;
import com.facebook.buck.step.fs.MakeCleanDirectoryStep;
@@ -48,8 +48,8 @@
import com.facebook.buck.util.DirectoryTraversal;
import com.facebook.buck.util.DirectoryTraverser;
import com.facebook.buck.util.MorePaths;
+import com.facebook.buck.util.ProjectFilesystem;
import com.google.common.base.Function;
-import com.google.common.base.Joiner;
import com.google.common.base.Optional;
import com.google.common.base.Predicates;
import com.google.common.base.Strings;
@@ -64,16 +64,12 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.List;
import java.util.Set;
public class AndroidBinaryRuleTest {
- /**
- * Directory where native libraries are expected to put their output.
- */
- final String nativeOutDir = "buck-out/bin/__native_zips__fbandroid_with_dash_debug_fbsign__/";
-
@Test
public void testAndroidBinaryNoDx() {
BuildRuleResolver ruleResolver = new BuildRuleResolver();
@@ -550,7 +546,7 @@
"/path/to/source",
"/path/to/destination/",
ImmutableList.of(
- "bash -c cp -R /path/to/source/* /path/to/destination/"));
+ "cp -R /path/to/source /path/to/destination"));
}
@Test
@@ -560,10 +556,8 @@
"/path/to/source",
"/path/to/destination/",
ImmutableList.of(
- "bash -c " +
- "[ -d /path/to/source/armeabi-v7a ] && " +
- "cp -R /path/to/source/armeabi-v7a /path/to/destination/ || " +
- "exit 0"));
+ "[ -d /path/to/source/armeabi-v7a ] && mkdir -p /path/to/destination/armeabi-v7a " +
+ "&& cp -R /path/to/source/armeabi-v7a /path/to/destination/armeabi-v7a"));
}
@Test
@@ -573,17 +567,41 @@
"/path/to/source",
"/path/to/destination/",
ImmutableList.of(
- "bash -c [ -d /path/to/source/armeabi ] && " +
- "cp -R /path/to/source/armeabi /path/to/destination/ || exit 0",
- "bash -c [ -d /path/to/source/x86 ] && " +
- "cp -R /path/to/source/x86 /path/to/destination/ || exit 0"));
+ "[ -d /path/to/source/armeabi ] && mkdir -p /path/to/destination/armeabi " +
+ "&& cp -R /path/to/source/armeabi /path/to/destination/armeabi",
+ "[ -d /path/to/source/x86 ] && mkdir -p /path/to/destination/x86 " +
+ "&& cp -R /path/to/source/x86 /path/to/destination/x86"));
}
private void createAndroidBinaryRuleAndTestCopyNativeLibraryCommand(
ImmutableSet<String> cpuFilters,
String sourceDir,
String destinationDir,
- ImmutableList<String> expectedShellCommands) {
+ ImmutableList<String> expectedCommandDescriptions) {
+
+ class FakeProjectFilesystem extends ProjectFilesystem {
+
+ public FakeProjectFilesystem() {
+ super(new File("."));
+ }
+
+ @Override
+ public Function<String, Path> getPathRelativizer() {
+ return new Function<String, Path>() {
+
+ @Override
+ public Path apply(String input) {
+ return Paths.get(input);
+ }
+ };
+ }
+
+ @Override
+ public Path resolve(Path path) {
+ return path;
+ }
+ }
+
BuildRuleResolver ruleResolver = new BuildRuleResolver();
AndroidBinaryRule.Builder builder = AndroidBinaryRule.newAndroidBinaryRuleBuilder(
new FakeAbstractBuildRuleBuilderParams())
@@ -602,14 +620,14 @@
ImmutableList<Step> steps = commands.build();
- assertEquals(steps.size(), expectedShellCommands.size());
+ assertEquals(steps.size(), expectedCommandDescriptions.size());
ExecutionContext context = createMock(ExecutionContext.class);
+ expect(context.getProjectFilesystem()).andReturn(new FakeProjectFilesystem()).anyTimes();
replay(context);
for (int i = 0; i < steps.size(); ++i) {
- Iterable<String> observedArgs = ((BashStep)steps.get(i)).getShellCommand(context);
- String observedCommand = Joiner.on(' ').join(observedArgs);
- assertEquals(expectedShellCommands.get(i), observedCommand);
+ String description = steps.get(i).getDescription(context);
+ assertEquals(expectedCommandDescriptions.get(i), description);
}
verify(context);
