sh> operator of the Digdag v0.10.0 on Windows platform
Relates to #1540
The sh> operator on the Digdag 0.10.0 doesn't work correctly on a Windows Platform.
@hundyhundy investigated this issue. This article was written in Japanese
In summary, It needs to change script name like runner.ps1 instead of runner.sh when Digdag uses PowerShell.
https://github.com/treasure-data/digdag/blob/42ea7cb71270957ea6424c2df88dd7c65d248789/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java#L136
Workaround: Windows user uses 0.9.42 until fix this issue.
Ref: twitter
This patch fixes this issue. Could you tell me what do you think this change? I believe the current behavior doesn't change anything.
If this patch seems good, what branch should I use for PR? (v0.11? or master)
diff --git a/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java b/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
index e6cf6f3608..8ff5540fde 100644
--- a/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
+++ b/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
@@ -133,14 +133,15 @@ public class ShOperatorFactory
{
final Path tempDir = workspace.createTempDir(String.format("digdag-sh-%d-", request.getTaskId()));
final Path workingDirectory = workspace.getPath(); // absolute
- final Path runnerPath = tempDir.resolve("runner.sh"); // absolute
+ final Boolean winOS = isWindowsPlatform();
+ final Path runnerPath = tempDir.resolve(winOS ? "runner.ps1" : "runner.sh"); // absolute
final List<String> shell;
if (params.has("shell")) {
shell = params.getListOrEmpty("shell", String.class);
}
else {
- shell = ImmutableList.of("/bin/sh");
+ shell = ImmutableList.of(winOS ? "PowerShell.exe" : "/bin/sh");
}
final ImmutableList.Builder<String> cmdline = ImmutableList.builder();
@@ -223,5 +224,14 @@ public class ShOperatorFactory
.ioDirectory(ioDirectory)
.build();
}
+ private boolean isWindowsPlatform()
+ {
+ final String os = System.getProperty("os.name").toLowerCase();
+ if ( os.startsWith("windows")) {
+ return true;
+ }
+ System.out.println("not windows");
+ return false;
+ }
}
}
Another idea. Add script_name option
diff --git a/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java b/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
index e6cf6f3608..57dcaddfca 100644
--- a/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
+++ b/digdag-standards/src/main/java/io/digdag/standards/operator/ShOperatorFactory.java
@@ -133,7 +133,7 @@ public class ShOperatorFactory
{
final Path tempDir = workspace.createTempDir(String.format("digdag-sh-%d-", request.getTaskId()));
final Path workingDirectory = workspace.getPath(); // absolute
- final Path runnerPath = tempDir.resolve("runner.sh"); // absolute
+ final Path runnerPath = tempDir.resolve(params.get("script_name", String.class, "runner.sh")); // absolute
final List<String> shell;
if (params.has("shell")) {
@szyn Could you comment about this issue?
@hiroyuki-sato Sorry, I overlooked your mention.
Thank you for raising and investigating this!
In my opinion, the former patch looks good to me. Could you create a PR for master branch when you have time?