Skip to content
Merged
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ $END
```

> [!NOTE]
> While command line processing can exploit all command line functionality (e.g., use environmental variables and wildcards in pathnames), this cannot be done in parameters' files.
> While command line processing can exploit all command line functionality (e.g., use environmental variables and wildcards in pathnames), this cannot be done in parameters' files. The exception are Shell jobs (`APP: SHELL`) that are run via a system shell. In such jobs wildcards and related shell features in `CMD` / `ARGS` are expanded by the shell. Other parameter values are still taken literally (they are not passed through a shell).

To use a parameters' file, call AutoCompChem and give it the pathname to the parameters' file as value of the `-p` (`--params`) argument:
```
Expand Down
126 changes: 98 additions & 28 deletions src/main/java/autocompchem/run/ShellJob.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,30 +22,36 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.JsonSerializationContext;
import com.google.gson.JsonSerializer;

import autocompchem.datacollections.NamedData;
import autocompchem.utils.StringUtils;
import autocompchem.utils.TimeUtils;

/**
* A shell job is work to be done by the shell. The shell command can be executed
* in a newly created subfolder. In this case any pathname should reflect the
* fact that `pwd` would return the pathname of the subfolder.
* <p>
* Commands are always launched via a system shell ({@code sh -c} or
* {@code cmd /c}) so that the shell can expand wildcards and other shell
* features in {@code CMD} / {@code ARGS} text. The {@code CMD} and
* {@code EXE}+{@code SCRIPT} parameter styles remain available; they only
* differ in how the command line string is assembled before it is handed to
* the shell.
*
* @author Marco Foscato
*/

public class ShellJob extends Job
{
/**
* The command will try to run
* The logical command components (for serialization / debugging).
* Execution always goes through a shell; see {@link #runThisJobSubClassSpecific()}.
*/
private List<String> command;

Expand Down Expand Up @@ -127,7 +133,8 @@ public Job makeInstance()
//------------------------------------------------------------------------------

/**
* Runs this SHELL command
* Runs this SHELL command via a system shell so that wildcards and related
* shell features in the command line are expanded by the interpreter.
*/

@Override
Expand All @@ -142,13 +149,14 @@ public void runThisJobSubClassSpecific()
+ "in a shell job. Use either one or the other.");
}

String commandLine = "";

// First we need to see if the command comes from the constructor or
// from parameter storage
if (params.contains(ShellJobConstants.LABINTERPRETER))
{
command = new ArrayList<String>();
command.add(params.getParameter(
ShellJobConstants.LABINTERPRETER).getValueAsString());
String interpreter = params.getParameter(
ShellJobConstants.LABINTERPRETER).getValueAsString();

if (!params.contains(ShellJobConstants.LABSCRIPT))
{
Expand All @@ -161,43 +169,58 @@ public void runThisJobSubClassSpecific()
ShellJobConstants.LABSCRIPT).getValueAsString();
script = script.replaceFirst("^~", System.getProperty("user.home"));
File scriptFile = getNewFile(script);
command.add(scriptFile.getAbsolutePath());
String scriptPath = scriptFile.getAbsolutePath();

command = new ArrayList<String>();
command.add(interpreter);
command.add(scriptPath);
// Quote EXE/SCRIPT so paths with spaces stay one word; leave
// ARGS raw so the shell can parse quotes and expand globs.
commandLine = shellQuote(interpreter) + " " + shellQuote(scriptPath);
} else if (params.contains(ShellJobConstants.LABCOMMAND))
{
String cmd = params.getParameter(
ShellJobConstants.LABCOMMAND).getValueAsString();
command = new ArrayList<String>();
Pattern regexMatchingArgs = Pattern.compile(
"[^\\s\"']+|\"[^\"]*\"|'[^']*'");
Matcher matcher = regexMatchingArgs.matcher(params.getParameter(
ShellJobConstants.LABCOMMAND).getValueAsString());
while (matcher.find())
{
command.add(matcher.group());
}
command.add(cmd);
// Raw CMD text: shell tokenizes and expands wildcards.
commandLine = cmd;
} else if (command != null && !command.isEmpty())
{
// Constructor-built components: quote each token so a single
// multi-word args component stays one argv entry (historical
// ProcessBuilder behaviour), while unquoted-safe globs still expand.
commandLine = command.stream()
.map(ShellJob::shellQuote)
.collect(Collectors.joining(" "));
}

if (params.contains(ShellJobConstants.LABARGS))
{
Pattern regexMatchingArgs = Pattern.compile(
"[^\\s\"']+|\"[^\"]*\"|'[^']*'");
Matcher matcher = regexMatchingArgs.matcher(params.getParameter(
ShellJobConstants.LABARGS).getValueAsString());
while (matcher.find())
String args = params.getParameter(
ShellJobConstants.LABARGS).getValueAsString();
if (command == null)
{
command = new ArrayList<String>();
}
command.add(args);
if (!commandLine.isEmpty())
{
command.add(matcher.group());
commandLine += " ";
}
// Raw ARGS: shell parses quotes and expands globs.
commandLine += args;
}

logger.info("Running " + appID + " Job: " + this.toString()
+ " Thread: " + Thread.currentThread().getName()
+ " " + TimeUtils.getTimestamp());

String commandAsString = StringUtils.mergeListToString(command, " ");

if (!commandAsString.trim().isEmpty())
if (commandLine != null && !commandLine.trim().isEmpty())
{
try
{
ProcessBuilder pb = new ProcessBuilder(command);
ProcessBuilder pb = new ProcessBuilder(wrapInShell(commandLine));
if (customUserDir != null)
{
// Here is where we move to the work space
Expand Down Expand Up @@ -249,13 +272,60 @@ public void runThisJobSubClassSpecific()
catch (Throwable t)
{
throw new RuntimeException("Error while running command line "
+ "operation '" + commandAsString + "'.", t);
+ "operation '" + commandLine + "'.", t);
}
}

logger.info("Done with " + appID + " Job " + this.toString() + " "
+ TimeUtils.getTimestamp());
}

//------------------------------------------------------------------------------

/**
* Wraps a command line so it is interpreted by a system shell.
* @param commandLine the full command line to run.
* @return argv for {@link ProcessBuilder}: shell, flag, command line.
*/
static List<String> wrapInShell(String commandLine)
{
if (isWindows())
{
return Arrays.asList("cmd.exe", "/c", commandLine);
}
return Arrays.asList("/bin/sh", "-c", commandLine);
}

//------------------------------------------------------------------------------

/**
* Quotes {@code s} for inclusion in a POSIX {@code sh -c} command line.
* Strings that contain only path-safe characters and glob metacharacters
* ({@code * ? [ ]}) are left unquoted so the shell can expand them.
* @param s the token to quote.
* @return a shell-safe token.
*/
static String shellQuote(String s)
{
if (s == null || s.isEmpty())
{
return "''";
}
// Unquoted: avoid spaces and shell metacharacters that are not globs
if (s.matches("[A-Za-z0-9_./:@%+=,\\-\\*\\?\\[\\]]+"))
{
return s;
}
return "'" + s.replace("'", "'\\''") + "'";
}

//------------------------------------------------------------------------------

private static boolean isWindows()
{
String os = System.getProperty("os.name");
return os != null && os.toLowerCase().contains("win");
}

//------------------------------------------------------------------------------

Expand Down
75 changes: 75 additions & 0 deletions src/test/java/autocompchem/run/ShellJobTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import java.io.File;
import java.io.FileWriter;
import java.nio.file.Files;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
Expand All @@ -33,6 +34,7 @@
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.io.TempDir;

import autocompchem.datacollections.ParameterStorage;
import autocompchem.files.FileAnalyzer;


Expand Down Expand Up @@ -107,6 +109,79 @@ public void testExposeShellOutputEnv() throws Exception
}
}

//------------------------------------------------------------------------------

@Test
@DisabledOnOs(WINDOWS)
public void testShellExpandsWildcardsInCmd() throws Exception
{
assertTrue(this.tempDir.isDirectory(), "Should be a directory");

Files.writeString(new File(tempDir, "alpha.txt").toPath(), "A");
Files.writeString(new File(tempDir, "beta.txt").toPath(), "B");
Files.writeString(new File(tempDir, "other.dat").toPath(), "X");

ParameterStorage params = new ParameterStorage();
params.setParameter(ShellJobConstants.LABCOMMAND,
"printf '%s\\n' *.txt");

ShellJob job = new ShellJob();
job.setParameters(params);
job.setUserDirAndStdFiles(tempDir);
job.setRedirectOutErr(true);
job.run();

File outFile = (File) job.getOutput("LOG").getValue();
String log = Files.readString(outFile.toPath());
assertTrue(log.contains("alpha.txt"), "Glob should match alpha.txt");
assertTrue(log.contains("beta.txt"), "Glob should match beta.txt");
assertFalse(log.contains("other.dat"), "Glob should not match other.dat");
}

//------------------------------------------------------------------------------

@Test
@DisabledOnOs(WINDOWS)
public void testShellExpandsWildcardsInExeArgs() throws Exception
{
assertTrue(this.tempDir.isDirectory(), "Should be a directory");

Files.writeString(new File(tempDir, "one.dat").toPath(), "1");
Files.writeString(new File(tempDir, "two.dat").toPath(), "2");

File script = new File(tempDir, "countargs.sh");
Files.writeString(script.toPath(),
"#!/bin/sh" + NL + "echo \"N=$#\"" + NL + "echo \"ALL=$*\"" + NL);

ParameterStorage params = new ParameterStorage();
params.setParameter(ShellJobConstants.LABINTERPRETER, "/bin/sh");
params.setParameter(ShellJobConstants.LABSCRIPT, script.getAbsolutePath());
params.setParameter(ShellJobConstants.LABARGS, "*.dat");

ShellJob job = new ShellJob();
job.setParameters(params);
job.setUserDirAndStdFiles(tempDir);
job.setRedirectOutErr(true);
job.run();

File outFile = (File) job.getOutput("LOG").getValue();
String log = Files.readString(outFile.toPath());
assertTrue(log.contains("N=2"), "Wildcard should expand to two args");
assertTrue(log.contains("one.dat"), "Should receive one.dat");
assertTrue(log.contains("two.dat"), "Should receive two.dat");
}

//------------------------------------------------------------------------------

@Test
public void testShellQuoteLeavesGlobsUnquoted()
{
assertEquals("*.txt", ShellJob.shellQuote("*.txt"));
assertEquals("file?.log", ShellJob.shellQuote("file?.log"));
assertEquals("'/path/with space'", ShellJob.shellQuote("/path/with space"));
assertEquals("''", ShellJob.shellQuote(""));
}

//------------------------------------------------------------------------------

}
8 changes: 4 additions & 4 deletions test/cli31.check
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ function not_passed() {

if [ ! -f "cli31.log" ] ; then not_passed $LINENO ; fi

if ! grep -q -i " varB=-2.5 " "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=-0.5 " "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=1.5 " "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=19.5 " "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=-2.5$" "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=-0.5$" "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=1.5$" "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varB=19.5$" "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varA=0.0 " "cli31.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varA=13.53 " "cli31.log" ; then not_passed $LINENO ; fi

Expand Down
6 changes: 3 additions & 3 deletions test/cli31.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
},
{
"reference": "CMD",
"value": "echo Iteration=ACCITERNUMBER varA=ACCVALEVARA varB=ACCVALEVARB ; exit -ACCITERNUMBER"
"value": "echo Iteration=ACCITERNUMBER varA=ACCVALEVARA varB=ACCVALEVARB ; exit -ACCITERINTNUMBER"
}
]
}
},
{
"reference": "replacementRules",
"value": "ACCITERNUMBER ${x}\nACCVALEVARA ${x*1.23}\nACCVALEVARB ${2*x - 2.5}"
"value": "ACCITERNUMBER ${x}\nACCITERINTNUMBER ${format('0', x)}\nACCVALEVARA ${x*1.23}\nACCVALEVARB ${2*x - 2.5}"
},
{
"reference": "maxIterations",
Expand All @@ -40,4 +40,4 @@
"value": "4"
}
]
}
}
2 changes: 1 addition & 1 deletion test/cli32.check
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ if ! grep -q -i "Iteration=11\.0 " "cli32.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varA=3\.87933" "cli32.log" ; then not_passed $LINENO ; fi
if ! grep -q -i " varA=14\.8793" "cli32.log" ; then not_passed $LINENO ; fi

n=0; n=$(grep -c -i " varB=\[4\.070.*, 4\.070.*\] " "cli32.log")
n=0; n=$(grep -c -i " varB=\[4\.070.*, 4\.070.*\]$" "cli32.log")
if [ 12 != "$n" ] ; then not_passed $LINENO ; exit 0 ; fi

n=0; n=$(grep -c -i "Initiating Shell Job" "cli32.log")
Expand Down
4 changes: 2 additions & 2 deletions test/cli32.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@
},
{
"reference": "CMD",
"value": "echo Iteration=ACCITERNUMBER varA=ACCVALEVARA varB=ACCVALEVARB ; exit -ACCITERNUMBER"
"value": "echo Iteration=ACCITERNUMBER varA=ACCVALEVARA varB=ACCVALEVARB ; exit -ACCITERINTNUMBER"
}
]
}
},
{
"reference": "replacementRules",
"value": "ACCITERNUMBER ${x}\nACCVALEVARA ${x + getACCJobsData(#-1.0,mol-0_RuRuDist,0)}\nACCVALEVARB getACCJobsData(#-1.0,mol-0_CCDist)"
"value": "ACCITERNUMBER ${x}\nACCITERINTNUMBER ${format('0', x)}\nACCVALEVARA ${x + getACCJobsData(#-1.0,mol-0_RuRuDist,0)}\nACCVALEVARB getACCJobsData(#-1.0,mol-0_CCDist)"
},
{
"reference": "maxIterations",
Expand Down
Loading
Loading