Skip to content

Commit

Permalink
Fix spaces handling in input/output files (#46)
Browse files Browse the repository at this point in the history
Surround file names with single quotes.

Signed-off-by: yichen88 <[email protected]>
  • Loading branch information
yichen88 authored and sylvlecl committed May 15, 2020
1 parent 10b8c0e commit 9b33760
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,31 @@ public String after(Path workingDir, ExecutionReport report) throws IOException
baseTest(supplier);
}

@Test
public void testFilesWithSpaces() {
Supplier<AbstractExecutionHandler<String>> supplier = () -> new AbstractExecutionHandler<String>() {
@Override
public List<CommandExecution> before(Path workingDir) {
generateZipFileOnRemote("in 0", workingDir.resolve("in 0.zip"));
generateZipFileOnRemote("in 1", workingDir.resolve("in 1.zip"));
generateZipFileOnRemote("in 2", workingDir.resolve("in 2.zip"));
return CommandExecutionsTestFactory.testFilesWithSpaces(3);
}

@Override
public String after(Path workingDir, ExecutionReport report) throws IOException {
super.after(workingDir, report);
Path out2 = workingDir.resolve("out 2.gz");
System.out.println("out 2.gz should exists, actual exists:" + Files.exists(out2));
if (Files.exists(out2)) {
return "OK";
}
return "KO";
}
};
baseTest(supplier);
}

@Test
public void testGroupCmd() {
Supplier<AbstractExecutionHandler<String>> supplier = () -> new AbstractReturnOKExecutionHandler() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Function;

/**
* Generates the content of the sbatch script, to be submitted using and {@link SbatchCmd}.
Expand Down Expand Up @@ -42,6 +43,9 @@ class SbatchScriptGenerator {
private static final String PRE_FILE = "PRE";
private static final String POST_FILE = "POST";

private static final Function<String, String> WRAP_FILENAME = str -> "'" + str + "'";
private static final Function<String, String> VAR2ARG = str -> "\"$" + str + "\"";

private final Path flagDir;

SbatchScriptGenerator(Path flagDir) {
Expand All @@ -54,7 +58,7 @@ List<String> unzipCommonInputFiles(Command command) {
command.getInputFiles()
.stream().filter(inputFile -> !inputFile.dependsOnExecutionNumber()) // common
.filter(inputFile -> inputFile.getPreProcessor() != null) // to unzip
.forEach(inputFile -> addUnzip(shell, inputFile.getName(0), inputFile.getPreProcessor()));
.forEach(inputFile -> addUnzipFilename(shell, inputFile.getName(0), inputFile.getPreProcessor()));
return shell;
}

Expand Down Expand Up @@ -99,7 +103,7 @@ List<String> parser(CommandExecution commandExecution, Path workingDir, Map<Stri
private void preProcess(List<String> list, Command command, int executionIndex) {
command.getInputFiles().stream()
.filter(InputFile::dependsOnExecutionNumber)
.forEach(file -> addUnzip(list, file.getName(executionIndex), file.getPreProcessor()));
.forEach(file -> addUnzipFilename(list, file.getName(executionIndex), file.getPreProcessor()));
}

private void preProcess(List<String> shell, Command command) {
Expand All @@ -110,19 +114,29 @@ private void preProcess(List<String> shell, Command command) {
// skip because this file is already unzip in a previous batch
continue;
}
addUnzip(shell, "$" + PRE_FILE + i, inputFile.getPreProcessor());
addUnzipVariable(shell, PRE_FILE + i, inputFile.getPreProcessor());
}
}

private static void addUnzip(List<String> shell, String filename, FilePreProcessor preProcessor) {
// used in batch mode
private static void addUnzipFilename(List<String> shell, String filename, FilePreProcessor preProcessor) {
addUnzipCmd(shell, WRAP_FILENAME.apply(filename), preProcessor);
}

// used in array mode
private static void addUnzipVariable(List<String> shell, String variableName, FilePreProcessor preProcessor) {
addUnzipCmd(shell, VAR2ARG.apply(variableName), preProcessor);
}

private static void addUnzipCmd(List<String> shell, String unzipArg, FilePreProcessor preProcessor) {
switch (preProcessor) {
case FILE_GUNZIP:
// gunzip the file
shell.add(SH_GUNZIP + filename);
shell.add(SH_GUNZIP + unzipArg);
break;
case ARCHIVE_UNZIP:
// extract the archive
shell.add(SH_UNZIP + filename);
shell.add(SH_UNZIP + unzipArg);
break;
default:
throw new AssertionError("Unexpected FilePreProcessor value: " + preProcessor);
Expand Down Expand Up @@ -171,7 +185,7 @@ private void postProcess(List<String> list, Command command, int executionIndex)
fileName = file.getName(executionIndex);
if (postProcessor != null) {
if (postProcessor == FilePostProcessor.FILE_GZIP) {
list.add(SH_GZIP + fileName);
list.add(SH_GZIP + WRAP_FILENAME.apply(fileName));
} else {
throw new AssertionError("Unexpected postProcessor type value: " + postProcessor);
}
Expand All @@ -184,15 +198,15 @@ private void postProcess(List<String> shell, Command command) {
for (int i = 0; i < outputFiles.size(); i++) {
OutputFile outputFile = outputFiles.get(i);
FilePostProcessor postProcessor = outputFile.getPostProcessor();
addGzip(shell, "$" + POST_FILE + i, postProcessor);
addGzipVariable(shell, POST_FILE + i, postProcessor);
// TODO add touch my error for gzip command??
}
}

private static void addGzip(List<String> shell, String fileName, FilePostProcessor postProcessor) {
private static void addGzipVariable(List<String> shell, String variableName, FilePostProcessor postProcessor) {
if (postProcessor != null) {
if (postProcessor == FilePostProcessor.FILE_GZIP) {
shell.add(SH_GZIP + fileName);
shell.add(SH_GZIP + VAR2ARG.apply(variableName));
} else {
throw new AssertionError("Unexpected postProcessor type value: " + postProcessor);
}
Expand Down Expand Up @@ -248,7 +262,7 @@ private void addInputFilenames(List<String> shell, int caseIdx, Command command)
// skip because this file is already unzip in a previous batch
continue;
}
ins.add(PRE_FILE + i + "=" + inputFile.getName(caseIdx));
ins.add(PRE_FILE + i + "=" + WRAP_FILENAME.apply(inputFile.getName(caseIdx)));
}
if (ins.isEmpty()) {
return;
Expand All @@ -264,7 +278,7 @@ private void addOutputFilenames(List<String> shell, int caseIdx, Command command
if (!outputFile.dependsOnExecutionNumber() || outputFile.getPostProcessor() == null) {
continue;
}
outs.add(POST_FILE + i + "=" + outputFile.getName(caseIdx));
outs.add(POST_FILE + i + "=" + WRAP_FILENAME.apply(outputFile.getName(caseIdx)));
}
if (outs.isEmpty()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,18 @@ static List<CommandExecution> myEchoSimpleCmdWithUnzipZip(int executionCount) {
return Collections.singletonList(commandExecution);
}

static List<CommandExecution> testFilesWithSpaces(int executionCount) {
Command command = new SimpleCommandBuilder()
.id("rename")
.program("mv")
.inputFiles(new InputFile(integer -> "in " + integer + ".zip", FilePreProcessor.ARCHIVE_UNZIP))
.outputFiles(new OutputFile(integer -> "out " + integer, FilePostProcessor.FILE_GZIP))
.args(i -> Arrays.asList("in " + i, "out " + i))
.build();
CommandExecution commandExecution = new CommandExecution(command, executionCount);
return Collections.singletonList(commandExecution);
}

static List<CommandExecution> commandFiles(int executionCount) {
InputFile stringInput = new InputFile("foo.zip", FilePreProcessor.ARCHIVE_UNZIP);
InputFile functionsInput = new InputFile(integer -> "in" + integer + ".zip", FilePreProcessor.ARCHIVE_UNZIP);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ public void testMyEchoSimpleCmd() {
Command command = commandExecution.getCommand();
List<String> shell = new SbatchScriptGenerator(flagPath).parser(command, 0, workingPath, Collections.emptyMap());
assertEquals(ImmutableList.of("#!/bin/sh",
"unzip -o -q in0.zip",
"unzip -o -q 'in0.zip'",
"/home/dev-itesla/myapps/myecho.sh \"in0\" \"out0\"",
"rc=$?; if [[ $rc != 0 ]]; then touch /tmp/flags/myerror_workingPath_12345_$SLURM_JOBID; exit $rc; fi",
"gzip out0",
"gzip 'out0'",
"touch /tmp/flags/mydone_workingPath_12345_$SLURM_JOBID"), shell);

assertCommandExecutionToShell(commandExecutions.get(0), "myEchoSimpleCmdWithUnzipZip.batch");
Expand All @@ -134,10 +134,10 @@ public void testCommandFiles() {
private static List<String> expectedTestCommandFilesBatch() {
List<String> shell = new ArrayList<>();
shell.add("#!/bin/sh");
shell.add("unzip -o -q in2.zip");
shell.add("unzip -o -q 'in2.zip'");
shell.add("/home/dev-itesla/myapps/myecho.sh \"in2\" \"out2\"");
shell.add("rc=$?; if [[ $rc != 0 ]]; then touch /tmp/flags/myerror_workingPath_12345_$SLURM_JOBID; exit $rc; fi");
shell.add("gzip tozip2");
shell.add("gzip 'tozip2'");
shell.add("touch /tmp/flags/mydone_workingPath_12345_$SLURM_JOBID");
return shell;
}
Expand All @@ -153,7 +153,7 @@ public void testOnlyUnzipBatch() {
private static List<String> expectedtestOnlyUnzipBatch() {
List<String> shell = new ArrayList<>();
shell.add("#!/bin/sh");
shell.add("unzip -o -q foo.zip");
shell.add("unzip -o -q 'foo.zip'");
return shell;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
#!/bin/sh
case $SLURM_ARRAY_TASK_ID in
0)
PRE0=in0.zip
POST0=out0
PRE0='in0.zip'
POST0='out0'
ARGS=("in0" "out0")
;;
1)
PRE0=in1.zip
POST0=out1
PRE0='in1.zip'
POST0='out1'
ARGS=("in1" "out1")
;;
2)
PRE0=in2.zip
POST0=out2
PRE0='in2.zip'
POST0='out2'
ARGS=("in2" "out2")
;;
esac
unzip -o -q $PRE0
unzip -o -q "$PRE0"
/home/dev-itesla/myapps/myecho.sh "${ARGS[@]}"
rc=$?; if [[ $rc != 0 ]]; then touch /tmp/flags/myerror_workingPath_12345_$SLURM_ARRAY_JOB_ID-$SLURM_ARRAY_TASK_ID; exit $rc; fi
gzip $POST0
gzip "$POST0"

0 comments on commit 9b33760

Please sign in to comment.