From d616165473c69a81f3e3d3f25aaa95cbb788bb45 Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Sat, 24 Feb 2024 21:55:39 +0900 Subject: [PATCH 1/2] Fix job execution on Windows platform --- .../standards/operator/ShOperatorFactory.java | 42 +++-- .../operator/ShOperatorFactoryTest.java | 151 ++++++++++++++++++ 2 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 digdag-standards/src/test/java/io/digdag/standards/operator/ShOperatorFactoryTest.java 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 aec2de7872..4d59eb76e2 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,23 +133,22 @@ private CommandStatus runCommand(final Config params, final CommandContext comma { 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(getScriptName()); // absolute final List shell; if (params.has("shell")) { - shell = params.getListOrEmpty("shell", String.class); + List temp_shell; + temp_shell = params.getListOrEmpty("shell", String.class); + shell = supportLegacyPowershellParm(temp_shell); } else { - shell = ImmutableList.of("/bin/sh"); + shell = ImmutableList.of(getDefaultShell()); } final ImmutableList.Builder cmdline = ImmutableList.builder(); - if (params.has("shell")) { - cmdline.addAll(shell); - } - else { - cmdline.addAll(shell); - } + + cmdline.addAll(shell); + cmdline.add(workingDirectory.relativize(runnerPath).toString()); // relative final String shScript = UserSecretTemplate.of(params.get("_command", String.class)) @@ -223,5 +222,30 @@ private CommandRequest buildCommandRequest(final CommandContext commandContext, .ioDirectory(ioDirectory) .build(); } + + private List supportLegacyPowershellParm (List temp_shell) + { + if (temp_shell.get(0).toLowerCase().equals("powershell.exe") && temp_shell.size() == 2){ + if (temp_shell.get(1).equals("-")){ + temp_shell.remove(1); + logger.warn("Deprecated shell entry: [\"powershell.exe\", \"-\"]"); + } + } + return temp_shell; + } + private String getScriptName() + { + return (isWindowsPlatform() ? "runner.ps1" : "runner.sh"); + } + private String getDefaultShell() + { + return (isWindowsPlatform() ? "powershell.exe" : "/bin/sh"); + } + + private boolean isWindowsPlatform() + { + final String os = System.getProperty("os.name").toLowerCase(); + return (os.startsWith("windows") ? true : false); + } } } diff --git a/digdag-standards/src/test/java/io/digdag/standards/operator/ShOperatorFactoryTest.java b/digdag-standards/src/test/java/io/digdag/standards/operator/ShOperatorFactoryTest.java new file mode 100644 index 0000000000..9083dc1e1d --- /dev/null +++ b/digdag-standards/src/test/java/io/digdag/standards/operator/ShOperatorFactoryTest.java @@ -0,0 +1,151 @@ +package io.digdag.standards.operator; + +import org.junit.After; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; +import org.omg.CORBA.ExceptionList; + +import com.fasterxml.jackson.databind.ObjectMapper; + +import java.nio.file.Path; +import java.util.ArrayList; + +import io.digdag.client.config.Config; +import io.digdag.client.config.ConfigFactory; +import io.digdag.core.DigdagEmbed; +import io.digdag.core.workflow.WorkflowTestingUtils; +import io.digdag.spi.TaskResult; +import io.digdag.standards.command.MockNonBlockingCommandExecutor; +import io.digdag.standards.operator.ShOperatorFactory; + +import static org.hamcrest.Matchers.is; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + + +import static io.digdag.core.workflow.OperatorTestingUtils.newOperatorFactory; +import static io.digdag.core.workflow.OperatorTestingUtils.newTaskRequest; +import static io.digdag.core.workflow.OperatorTestingUtils.newContext; +import static io.digdag.core.workflow.WorkflowTestingUtils.loadYamlResource; +import static io.digdag.core.workflow.WorkflowTestingUtils.setupEmbed; +import static io.digdag.core.workflow.WorkflowTestingUtils.runWorkflow; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.util.List; + +public class ShOperatorFactoryTest +{ + private static DigdagEmbed embed; + + @BeforeClass + public static void createDigdagEmbed() + { + embed = WorkflowTestingUtils.setupEmbed(); + } + + @Rule + public TemporaryFolder folder = new TemporaryFolder(); + + private ObjectMapper mapper; + private MockNonBlockingCommandExecutor executor; + private Path tempPath; + + @Before + public void createInstance1() + { + this.mapper = embed.getInjector().getInstance(ObjectMapper.class); + this.executor = new MockNonBlockingCommandExecutor(mapper); + this.tempPath = folder.getRoot().toPath(); + //System.setProperty("os.name","windows"); + + } + + @Test + public void testGetShellAndScriptnameOnWin() + throws Exception + { + System.setProperty("os.name","windows"); + + final String configResource = "/io/digdag/standards/operator/sh/basic.yml"; + final ShOperatorFactory operatorFactory = new ShOperatorFactory(executor); + final ShOperatorFactory.ShOperator operator = (ShOperatorFactory.ShOperator) operatorFactory.newOperator( + newContext(tempPath, newTaskRequest().withConfig(loadYamlResource(configResource)))); + + + Method method_isWindowsPlatform = ShOperatorFactory.ShOperator.class.getDeclaredMethod("isWindowsPlatform"); + method_isWindowsPlatform.setAccessible(true); + assertTrue((boolean)method_isWindowsPlatform.invoke(operator)); + + Method method_getDefaultShell = ShOperatorFactory.ShOperator.class.getDeclaredMethod("getDefaultShell"); + method_getDefaultShell.setAccessible(true); + assertTrue("powershell.exe" == (String)method_getDefaultShell.invoke(operator)); + + Method method_getScriptName = ShOperatorFactory.ShOperator.class.getDeclaredMethod("getScriptName"); + method_getScriptName.setAccessible(true); + assertTrue("runner.ps1" == (String)method_getScriptName.invoke(operator)); + + System.clearProperty("os.name"); + } + + + @Test + public void testGetShellAndScriptnameOnLinux() + throws Exception + { + System.setProperty("os.name","linux"); + + final String configResource = "/io/digdag/standards/operator/sh/basic.yml"; + final ShOperatorFactory operatorFactory = new ShOperatorFactory(executor); + final ShOperatorFactory.ShOperator operator = (ShOperatorFactory.ShOperator) operatorFactory.newOperator( + newContext(tempPath, newTaskRequest().withConfig(loadYamlResource(configResource)))); + + Method method = ShOperatorFactory.ShOperator.class.getDeclaredMethod("isWindowsPlatform"); + method.setAccessible(true); + assertFalse((boolean)method.invoke(operator)); + + Method method_getDefaultShell = ShOperatorFactory.ShOperator.class.getDeclaredMethod("getDefaultShell"); + method_getDefaultShell.setAccessible(true); + assertTrue("/bin/sh" == (String)method_getDefaultShell.invoke(operator)); + + Method method_getScriptName = ShOperatorFactory.ShOperator.class.getDeclaredMethod("getScriptName"); + method_getScriptName.setAccessible(true); + assertTrue("runner.sh" == (String)method_getScriptName.invoke(operator)); + + System.clearProperty("os.name"); + } + + + @Test + public void testSupportLegacyPowershellParm() + throws Exception + { + final String configResource = "/io/digdag/standards/operator/sh/basic.yml"; + final ShOperatorFactory operatorFactory = new ShOperatorFactory(executor); + final ShOperatorFactory.ShOperator operator = (ShOperatorFactory.ShOperator) operatorFactory.newOperator( + newContext(tempPath, newTaskRequest().withConfig(loadYamlResource(configResource)))); + + Method method = ShOperatorFactory.ShOperator.class.getDeclaredMethod("supportLegacyPowershellParm", List.class); + method.setAccessible(true); + + List legacy_powershell_parm = new ArrayList(){{add("powershell.exe");add("-");}}; + List powershell_parm = (List)method.invoke(operator, legacy_powershell_parm); + assertTrue(1 == powershell_parm.size()); + assertTrue("powershell.exe" == powershell_parm.get(0).toLowerCase()); + + } + + +} + + From b205dbec92249952f93181968f59ded797281567 Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Sun, 25 Feb 2024 09:47:05 +0900 Subject: [PATCH 2/2] Fix docs --- digdag-docs/src/operators/sh.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/digdag-docs/src/operators/sh.md b/digdag-docs/src/operators/sh.md index 5c37a4b24b..eb36c6e896 100644 --- a/digdag-docs/src/operators/sh.md +++ b/digdag-docs/src/operators/sh.md @@ -37,7 +37,7 @@ On Windows, you can set PowerShell.exe to the `shell` option: _export: sh: - shell: ["powershell.exe", "-"] + shell: ["powershell.exe"] +step1: sh>: step1.exe