From b12a4fe863c2f2dbc8d512763c17ca4a259270a4 Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Fri, 5 May 2023 09:25:59 +0900 Subject: [PATCH 1/7] fix: Fix for Windows environment - In Windows environment, set the extension of the executable script to .ps1. - When specifying "powershell" for shell in a .dig file, the specification ["powershell.exe","-"] was used until digdag ver. 0.9.x. Since ver. 0.10, the "-" is no longer necessary. However, to ensure compatibility, the specification ["powershell.exe","-"] should be read as ["powershell.exe"]. --- .../standards/operator/ShOperatorFactory.java | 41 +++++++++++++++---- 1 file changed, 33 insertions(+), 8 deletions(-) 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..1ad217766f 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 @@ -128,20 +128,34 @@ private void runCode(final Config state) } } + private List getShell(Config params, Boolean winOS){ + // Until digdag ver. 0.9, it was necessary to write ["powershell.exe","-"] to specify shell, + // but since ver. 0.10, jobs are not executed unless ["powershell.exe"] is written. + // To resolve this incompatibility, change the shell specification ["powershell.exe","-"] + // to ["powershell.exe"]. + List temp_shell; + if (params.has("shell")) { + temp_shell = params.getListOrEmpty("shell", String.class); + if (temp_shell.get(0).toLowerCase().contains("powershell") && temp_shell.size() == 2){ + temp_shell.remove(1); + logger.info("removed \"-\" from shell specification --- from [\"powershell.exe\",\"-\"] to [\"powershell.exe\"] ."); + } + } + else { + temp_shell = ImmutableList.of(winOS ? "PowerShell.exe" : "/bin/sh"); + } + return temp_shell; + } + private CommandStatus runCommand(final Config params, final CommandContext commandContext) throws IOException, InterruptedException { 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 shell; - if (params.has("shell")) { - shell = params.getListOrEmpty("shell", String.class); - } - else { - shell = ImmutableList.of("/bin/sh"); - } + final List shell = getShell(params, winOS); final ImmutableList.Builder cmdline = ImmutableList.builder(); if (params.has("shell")) { @@ -223,5 +237,16 @@ private CommandRequest buildCommandRequest(final CommandContext commandContext, .ioDirectory(ioDirectory) .build(); } + + private boolean isWindowsPlatform() + { + final String os = System.getProperty("os.name").toLowerCase(); + if ( os.startsWith("windows")) { + System.out.println("windows!!"); + return true; + } + System.out.println("not windows"); + return false; + } } } From e69d8b3c273d3d2c1b8eb910d4c4256fd454addd Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Fri, 5 May 2023 10:01:08 +0900 Subject: [PATCH 2/7] fix: Remove println for debug --- .../java/io/digdag/standards/operator/ShOperatorFactory.java | 2 -- 1 file changed, 2 deletions(-) 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 1ad217766f..95990b4eee 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 @@ -242,10 +242,8 @@ private boolean isWindowsPlatform() { final String os = System.getProperty("os.name").toLowerCase(); if ( os.startsWith("windows")) { - System.out.println("windows!!"); return true; } - System.out.println("not windows"); return false; } } From 86461b849f65b42210cecb1dc1eb4c7a135899cd Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Fri, 5 May 2023 10:36:01 +0900 Subject: [PATCH 3/7] fix: Fix document --- digdag-docs/src/operators/sh.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/digdag-docs/src/operators/sh.md b/digdag-docs/src/operators/sh.md index 5c37a4b24b..bdde105e25 100644 --- a/digdag-docs/src/operators/sh.md +++ b/digdag-docs/src/operators/sh.md @@ -33,7 +33,23 @@ The shell defaults to `/bin/sh`. If an alternate shell such as `zsh` is desired, +step1: sh>: tasks/step2.sh -On Windows, you can set PowerShell.exe to the `shell` option: +On Windows, you can set PowerShell.exe to the `shell` option. +Since ver.0.10.6, it is correct to specify ["powershell.exe"], +but digdag works with ["powershell.exe", "-"] in ver.0.9.46 +or earlier. + +>= 0.10.6 + _export: + sh: + shell: ["powershell.exe"] + + +step1: + sh>: step1.exe + + +step2: + sh>: step2.ps1 + +<= 0.9.46 _export: sh: From 048948cc8f9a4c62e1c03b5cf6caba18f25cf45b Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Fri, 5 May 2023 18:10:04 +0900 Subject: [PATCH 4/7] fix: Delete duplicate lines --- .../io/digdag/standards/operator/ShOperatorFactory.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 95990b4eee..30a326cf98 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 @@ -158,12 +158,9 @@ private CommandStatus runCommand(final Config params, final CommandContext comma final List shell = getShell(params, winOS); 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)) From 37817be91be4ea037c285f28d69d45930024f978 Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Fri, 5 May 2023 19:16:36 +0900 Subject: [PATCH 5/7] fix: Add "final" to variable definitions --- .../java/io/digdag/standards/operator/ShOperatorFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 30a326cf98..87375b205d 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 @@ -128,7 +128,7 @@ private void runCode(final Config state) } } - private List getShell(Config params, Boolean winOS){ + private List getShell(final Config params, final Boolean winOS){ // Until digdag ver. 0.9, it was necessary to write ["powershell.exe","-"] to specify shell, // but since ver. 0.10, jobs are not executed unless ["powershell.exe"] is written. // To resolve this incompatibility, change the shell specification ["powershell.exe","-"] From 922b5339f00966960491ae9cf5fc26795386500c Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Mon, 17 Jul 2023 14:34:13 +0900 Subject: [PATCH 6/7] fix: More precise confirmation of shell designations --- .../io/digdag/standards/operator/ShOperatorFactory.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) 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 87375b205d..b322bf37e7 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 @@ -137,8 +137,10 @@ private List getShell(final Config params, final Boolean winOS){ if (params.has("shell")) { temp_shell = params.getListOrEmpty("shell", String.class); if (temp_shell.get(0).toLowerCase().contains("powershell") && temp_shell.size() == 2){ - temp_shell.remove(1); - logger.info("removed \"-\" from shell specification --- from [\"powershell.exe\",\"-\"] to [\"powershell.exe\"] ."); + if (temp_shell.get(1).contains("-")){ + temp_shell.remove(1); + logger.info("removed \"-\" from shell specification --- from [\"powershell.exe\",\"-\"] to [\"powershell.exe\"] ."); + } } } else { From d1e7447b61b952d889b593fe31b3b0e5d9fa5396 Mon Sep 17 00:00:00 2001 From: Hiroshi KONO Date: Mon, 18 Sep 2023 11:40:24 +0900 Subject: [PATCH 7/7] fix: Check shell options to be converted more closely --- .../digdag/standards/operator/ShOperatorFactory.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) 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 b322bf37e7..f8ad15aff3 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 @@ -131,15 +131,16 @@ private void runCode(final Config state) private List getShell(final Config params, final Boolean winOS){ // Until digdag ver. 0.9, it was necessary to write ["powershell.exe","-"] to specify shell, // but since ver. 0.10, jobs are not executed unless ["powershell.exe"] is written. - // To resolve this incompatibility, change the shell specification ["powershell.exe","-"] - // to ["powershell.exe"]. + // To cope with the sudden incompatibility, change the shell specification ["powershell.exe","-"] + // to ["powershell.exe"]. + // In addition, a deprecation message is output. This conversion will be removed in the future. List temp_shell; if (params.has("shell")) { temp_shell = params.getListOrEmpty("shell", String.class); - if (temp_shell.get(0).toLowerCase().contains("powershell") && temp_shell.size() == 2){ - if (temp_shell.get(1).contains("-")){ + if (temp_shell.get(0).toLowerCase().equals("powershell.exe") && temp_shell.size() == 2){ + if (temp_shell.get(1).equals("-")){ temp_shell.remove(1); - logger.info("removed \"-\" from shell specification --- from [\"powershell.exe\",\"-\"] to [\"powershell.exe\"] ."); + logger.info("Removed \"-\" from shell specification --- from [\"powershell.exe\",\"-\"] to [\"powershell.exe\"] ."); } } }