You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2021/07/20 14:49:30 UTC
[GitHub] [hadoop] tomicooler opened a new pull request #3220: Feature/yarn 10355
tomicooler opened a new pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220
## NOTICE
Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HADOOP-XXXXX. Fix a typo in YYY.)
For more details, please see https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] szilard-nemeth commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
szilard-nemeth commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892539187
Thanks @tomicooler for working on this.
Nice patch, LGTM +1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r674760077
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1277,6 +1279,36 @@ void resolve() {
}
private static final class UnixShellScriptBuilder extends ShellScriptBuilder {
+ // Visualization for the regex: https://regex101.com/r/j7yaU1/1
Review comment:
I replaced the regex101 link with a new one, I created an account so I will be able to update it later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892548695
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 30m 41s | | trunk passed |
| +1 :green_heart: | compile | 1m 27s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 24s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 35s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 48s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 37s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 23s | | trunk passed |
| +1 :green_heart: | shadedclient | 14m 32s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 1m 21s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 1m 21s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 1s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 25s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 37s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 14m 31s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 0s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 97m 46s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 972945f10ef6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d8334704611b30af3400e8978a3c0506dec46c3c |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/testReport/ |
| Max. process+thread count | 546 (vs. ulimit of 5500) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] brumi1024 commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
brumi1024 commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673999244
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,64 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
Review comment:
I think the line length limit could be omitted here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
shuzirra commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r676708584
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1277,6 +1279,36 @@ void resolve() {
}
private static final class UnixShellScriptBuilder extends ShellScriptBuilder {
+ // Visualization for the regex: https://regex101.com/r/Q7DxG1/1
+ private static final String VARIABLE_NAME = "variableName";
+ private static final String SHELL_EXPANSION = "shellExpansion";
+ private static final String DOUBLE_QUOTED = "doubleQuoted";
+ private static final String SINGLE_QUOTED = "singleQuoted";
+
+ private static final String PATTERN_VARIABLE_NAME =
+ "\\$(?<" + VARIABLE_NAME + ">[_a-zA-Z][_a-zA-Z0-9]*)";
+ private static final String PATTERN_NO_ESCAPE = "(?<![\\\\])";
+ private static final String PATTERN_SHELL_EXPANSION =
+ "\\$\\{(?<" + SHELL_EXPANSION +
+ ">[_a-zA-Z#][_a-zA-Z0-9\\[\\]\\*\\:\\-\\$#=\\?+/%^,@]*)\\}";
+ private static final String PATTERN_DOUBLE_QUOTED =
+ "\"(?<" + DOUBLE_QUOTED + ">.*?[^\\\\])\"";
+ private static final String PATTERN_SINGLE_QUOTED =
+ "'(?<" + SINGLE_QUOTED + ">.*?[^\\\\])'";
+ private static final String PATTERN_SPLIT_SHELL_EXPANSION =
+ "[^_a-zA-Z0-9]";
Review comment:
Please add java doc comments where you explain what the given regex matches. Also examples can help. It's quite hard to follow what's going on here, and if we are making a refactor for readability then let's make it as readable as possible.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1277,6 +1279,36 @@ void resolve() {
}
private static final class UnixShellScriptBuilder extends ShellScriptBuilder {
+ // Visualization for the regex: https://regex101.com/r/Q7DxG1/1
+ private static final String VARIABLE_NAME = "variableName";
+ private static final String SHELL_EXPANSION = "shellExpansion";
+ private static final String DOUBLE_QUOTED = "doubleQuoted";
+ private static final String SINGLE_QUOTED = "singleQuoted";
+
+ private static final String PATTERN_VARIABLE_NAME =
+ "\\$(?<" + VARIABLE_NAME + ">[_a-zA-Z][_a-zA-Z0-9]*)";
+ private static final String PATTERN_NO_ESCAPE = "(?<![\\\\])";
+ private static final String PATTERN_SHELL_EXPANSION =
+ "\\$\\{(?<" + SHELL_EXPANSION +
+ ">[_a-zA-Z#][_a-zA-Z0-9\\[\\]\\*\\:\\-\\$#=\\?+/%^,@]*)\\}";
+ private static final String PATTERN_DOUBLE_QUOTED =
+ "\"(?<" + DOUBLE_QUOTED + ">.*?[^\\\\])\"";
+ private static final String PATTERN_SINGLE_QUOTED =
+ "'(?<" + SINGLE_QUOTED + ">.*?[^\\\\])'";
+ private static final String PATTERN_SPLIT_SHELL_EXPANSION =
+ "[^_a-zA-Z0-9]";
+
+ private final Pattern mainPattern = Pattern
+ .compile("((" + PATTERN_NO_ESCAPE + PATTERN_VARIABLE_NAME + ")|(" +
+ PATTERN_NO_ESCAPE + PATTERN_SHELL_EXPANSION + ")|(" +
+ PATTERN_DOUBLE_QUOTED + "))");
Review comment:
A few examples here can also help, just to see what this regexp is looking for.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1449,6 +1451,9 @@ public void setExitOnFailure() {
private static final class WindowsShellScriptBuilder
extends ShellScriptBuilder {
+ private static final String PATTERN_VARIABLE = "%(.*?)%";
+ private Pattern pattern;
Review comment:
Can be static an intitialized here, so we don't have to do the null check each time in getEnvDependencies.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,68 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
Review comment:
Please add a javadoc comment explaining what this method does, and a few examples as well, unfortunately the fillDeps is not expressive enough, and the method is too complex to understand it just by looking at it.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1544,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile(PATTERN_VARIABLE);
+ }
Review comment:
Can be replaced by initializing the field pattern, see my comment above.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,68 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html
+ /*
+ Matching shell expansion variables, e.g.:
+ - ${variableName}
+ - ${foo:-$bar}
+ - ${#bar}
+ - etc
+ */
+ String shellExpansion = matcher.group(SHELL_EXPANSION);
+ if (shellExpansion != null && !shellExpansion.isEmpty()) {
+ if (shellExpansion.charAt(0) == '#') {
+ shellExpansion = shellExpansion.substring(1);
}
- c = envVal.charAt(i);
- if (c == '{') { // for ${... bash like syntax
- i++;
- if (i >= len) {
- break;
- }
- c = envVal.charAt(i);
- if (c == '#') { // for ${#... bash array syntax
- i++;
- if (i >= len) {
- break;
- }
- }
+ String[] split =
+ shellExpansionSplitPattern.split(shellExpansion, 2);
Review comment:
Please add comment and example here as well
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1544,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile(PATTERN_VARIABLE);
+ }
+ Matcher matcher = pattern.matcher(envVal);
final Set<String> deps = new HashSet<>();
- final int len = envVal.length();
- int i = 0;
- while (i < len) {
- i = envVal.indexOf('%', i); // find beginning of variable
- if (i < 0 || i == (len - 1)) {
- break;
- }
- i++;
- // 3 cases: %var%, %var:...% or %%
- final int j = envVal.indexOf('%', i); // find end of variable
- if (j == i) {
- // %% case, just skip it
- i++;
- continue;
- }
- if (j < 0) {
- break; // even %var:...% syntax ends with a %, so j cannot be negative
- }
- final int k = envVal.indexOf(':', i);
- if (k >= 0 && k < j) {
- // %var:...% syntax
- deps.add(envVal.substring(i, k));
- } else {
- // %var% syntax
- deps.add(envVal.substring(i, j));
+ while (matcher.find()) {
+ String match = matcher.group(1);
+ if (match.length() > 0) {
+ String[] split = match.split(":", 2);
+ deps.add(split[0]);
Review comment:
Much cleaner than the previous version, but let's add a few comments about the expected variable formats (%var%, %%, %a:b%), because the split along ':' is a bit confusing without any comments (and without the previous version of the code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] brumi1024 commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
brumi1024 commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-891914615
@tomicooler checked the latest state, other than the two small checkstyle issues +1 from my side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-884190700
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 11m 17s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 20m 0s | | trunk passed |
| +1 :green_heart: | compile | 21m 19s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 19m 54s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 3m 39s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 37s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 18s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 15s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 33s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 15m 38s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 53s | | the patch passed |
| +1 :green_heart: | compile | 23m 21s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 23m 21s | | the patch passed |
| +1 :green_heart: | compile | 19m 13s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 19m 13s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 42s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 38s | | the patch passed |
| +1 :green_heart: | xml | 0m 2s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 16s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 18s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 36s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 15m 34s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 30s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 23m 30s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 59s | | The patch does not generate ASF License warnings. |
| | | 195m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux 69da8b8d779b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 279c7b6626661966da74f56637828042a93a7d99 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/3/testReport/ |
| Max. process+thread count | 548 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/3/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] brumi1024 commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
brumi1024 commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r674000662
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
Review comment:
Same here about the length.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-888301811
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 4s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 12m 35s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 23m 8s | | trunk passed |
| +1 :green_heart: | compile | 24m 2s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 20m 38s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 3m 59s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 32s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 20s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 17s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 34s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 19m 27s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 29s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 27m 34s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 27m 34s | | the patch passed |
| +1 :green_heart: | compile | 23m 48s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 23m 48s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 4m 15s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 29s | | the patch passed |
| +1 :green_heart: | xml | 0m 2s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 28s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 28s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 20m 10s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 29s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 24m 32s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 58s | | The patch does not generate ASF License warnings. |
| | | 223m 8s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux 00e7d0292931 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 8251f55ee2b5150dab9fc59709996908ceaeb37c |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/6/testReport/ |
| Max. process+thread count | 574 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/6/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673926349
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_LINUX);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Stream<Arguments> inputForGetEnvDependenciesWin() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("%%%%%%", asSet()),
+ Arguments.of("%%A%", asSet()),
+ Arguments.of("%A", asSet()),
+ Arguments.of("%A:", asSet()),
+ Arguments.of("%A%", asSet("A")),
+ Arguments.of("%%%A%", asSet("A")),
+ Arguments.of("%%C%A%", asSet("A")),
+ Arguments.of("%A:~-1%", asSet("A")),
+ Arguments.of("%A:%", asSet("A")),
+ Arguments.of("%A:whatever:a:b:%", asSet("A")),
+ Arguments.of("%A%B%", asSet("A")),
+ Arguments.of("%A%%%%%B%", asSet("A")),
+ Arguments.of("%A%%B%", asSet("A", "B")),
+ Arguments.of("%A%%%%B%", asSet("A", "B")),
+ Arguments.of("%A%:%B%:%C%:pathlist var", asSet("A", "B", "C")),
+ Arguments.of("%A%\\\\foo\\\\bar:%B%:%C%:pathlist var",
+ asSet("A", "B", "C"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesWin")
+ void testGetEnvDependenciesWin(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_WIN);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Set<String> asSet(String... str) {
Review comment:
hadoop-yarn-server-nodemanager does not depend on Guava yet
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
shuzirra commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892657250
@tomicooler thak you for the patch, LGTM+1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
shuzirra commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892657250
@tomicooler thak you for the patch, LGTM+1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-885066503
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 8s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 13m 1s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 29m 4s | | trunk passed |
| +1 :green_heart: | compile | 30m 48s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 25m 24s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 5m 24s | | trunk passed |
| +1 :green_heart: | mvnsite | 2m 3s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 47s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 43s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 50s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 22m 34s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 31s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 1m 2s | | the patch passed |
| +1 :green_heart: | compile | 29m 35s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 29m 35s | | the patch passed |
| +1 :green_heart: | compile | 25m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 25m 28s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 4m 37s | [/results-checkstyle-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/5/artifact/out/results-checkstyle-root.txt) | root: The patch generated 2 new + 140 unchanged - 0 fixed = 142 total (was 140) |
| +1 :green_heart: | mvnsite | 1m 39s | | the patch passed |
| +1 :green_heart: | xml | 0m 3s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 29s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 25s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 33s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 21m 32s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 31s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 25m 15s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 57s | | The patch does not generate ASF License warnings. |
| | | 254m 46s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux 46422e5023a0 4.15.0-128-generic #131-Ubuntu SMP Wed Dec 9 06:57:35 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d7410b0833ae45581c6a4cc8f61c87c778d423a0 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/5/testReport/ |
| Max. process+thread count | 525 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/5/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-884040951
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 11m 33s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 23m 30s | | trunk passed |
| +1 :green_heart: | compile | 25m 15s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 19m 42s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 3m 50s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 42s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 34s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 28s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 43s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 15m 0s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 30s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 52s | | the patch passed |
| +1 :green_heart: | compile | 20m 26s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 20m 26s | | the patch passed |
| +1 :green_heart: | compile | 18m 17s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 18m 17s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 45s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 39s | | the patch passed |
| +1 :green_heart: | xml | 0m 2s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 28s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 30s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 39s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 15m 12s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 36s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 23m 2s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 58s | | The patch does not generate ASF License warnings. |
| | | 200m 17s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux 530612d41682 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 3665093b49a51999400214f566a4bc31e1d66530 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/2/testReport/ |
| Max. process+thread count | 677 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/2/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673911895
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ String variableName = matcher.group(VARIABLE_NAME);
+ if (variableName != null && !variableName.isEmpty()) {
+ deps.add(variableName);
+ }
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ fillDeps(deps, doubleQuoted);
+ }
+
+ String shellExpansion = matcher.group(SHELL_EXPANSION);
+ if (shellExpansion != null && !shellExpansion.isEmpty()) {
+ if (shellExpansion.charAt(0) == '#') {
+ shellExpansion = shellExpansion.substring(1);
}
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ String[] split = shellExpansion.split("[:#%/^,@\\[\\]]", 2);
Review comment:
I extracted and made a Pattern, so the regex won't be compiled every time. I also changed the regex pattern to split at any special character the variable name can't contain. That is more close to the original code's logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673912573
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
Review comment:
Added some explanation in comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] szilard-nemeth commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
szilard-nemeth commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892539187
Thanks @tomicooler for working on this.
Nice patch, LGTM +1.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892548695
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 38s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 30m 41s | | trunk passed |
| +1 :green_heart: | compile | 1m 27s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 24s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 35s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 48s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 40s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 37s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 23s | | trunk passed |
| +1 :green_heart: | shadedclient | 14m 32s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 1m 21s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 1m 21s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 1s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 25s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 37s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 14m 31s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 0s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 97m 46s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 972945f10ef6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / d8334704611b30af3400e8978a3c0506dec46c3c |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/testReport/ |
| Max. process+thread count | 546 (vs. ulimit of 5500) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/9/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] brumi1024 commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
brumi1024 commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-886725956
Thanks for the updates, +1 from my side as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra merged pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
shuzirra merged pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] brumi1024 commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
brumi1024 commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673992746
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1277,6 +1279,36 @@ void resolve() {
}
private static final class UnixShellScriptBuilder extends ShellScriptBuilder {
+ // Visualization for the regex: https://regex101.com/r/j7yaU1/1
Review comment:
Nice touch on the visualisation link, one question though: will this be available in a year or two?
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
Review comment:
Same here about the width.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,64 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
Review comment:
I think the line width limit could be omitted here.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,64 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ // matching shell expansion variables, e.g.:
+ // ${variableName} ${foo:-$bar} ${#bar} etc
Review comment:
Nit: multiline comment could be used.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_LINUX);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Stream<Arguments> inputForGetEnvDependenciesWin() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("%%%%%%", asSet()),
+ Arguments.of("%%A%", asSet()),
+ Arguments.of("%A", asSet()),
+ Arguments.of("%A:", asSet()),
+ Arguments.of("%A%", asSet("A")),
+ Arguments.of("%%%A%", asSet("A")),
+ Arguments.of("%%C%A%", asSet("A")),
+ Arguments.of("%A:~-1%", asSet("A")),
+ Arguments.of("%A:%", asSet("A")),
+ Arguments.of("%A:whatever:a:b:%", asSet("A")),
+ Arguments.of("%A%B%", asSet("A")),
+ Arguments.of("%A%%%%%B%", asSet("A")),
+ Arguments.of("%A%%B%", asSet("A", "B")),
+ Arguments.of("%A%%%%B%", asSet("A", "B")),
+ Arguments.of("%A%:%B%:%C%:pathlist var", asSet("A", "B", "C")),
+ Arguments.of("%A%\\\\foo\\\\bar:%B%:%C%:pathlist var",
+ asSet("A", "B", "C"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesWin")
+ void testGetEnvDependenciesWin(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_WIN);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Set<String> asSet(String... str) {
Review comment:
But org.apache.hadoop.util.Sets should contain a method like this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r674763308
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_LINUX);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Stream<Arguments> inputForGetEnvDependenciesWin() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("%%%%%%", asSet()),
+ Arguments.of("%%A%", asSet()),
+ Arguments.of("%A", asSet()),
+ Arguments.of("%A:", asSet()),
+ Arguments.of("%A%", asSet("A")),
+ Arguments.of("%%%A%", asSet("A")),
+ Arguments.of("%%C%A%", asSet("A")),
+ Arguments.of("%A:~-1%", asSet("A")),
+ Arguments.of("%A:%", asSet("A")),
+ Arguments.of("%A:whatever:a:b:%", asSet("A")),
+ Arguments.of("%A%B%", asSet("A")),
+ Arguments.of("%A%%%%%B%", asSet("A")),
+ Arguments.of("%A%%B%", asSet("A", "B")),
+ Arguments.of("%A%%%%B%", asSet("A", "B")),
+ Arguments.of("%A%:%B%:%C%:pathlist var", asSet("A", "B", "C")),
+ Arguments.of("%A%\\\\foo\\\\bar:%B%:%C%:pathlist var",
+ asSet("A", "B", "C"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesWin")
+ void testGetEnvDependenciesWin(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_WIN);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Set<String> asSet(String... str) {
Review comment:
Replaced with the hadoop.utils.Sets alternative.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-884225782
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 35s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 12m 39s | | Maven dependency ordering for branch |
| -1 :x: | mvninstall | 21m 10s | [/branch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/4/artifact/out/branch-mvninstall-root.txt) | root in trunk failed. |
| +1 :green_heart: | compile | 22m 16s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 20m 30s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 3m 56s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 38s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 22s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 13s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 34s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 15m 25s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 56s | | the patch passed |
| +1 :green_heart: | compile | 21m 40s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 21m 40s | | the patch passed |
| +1 :green_heart: | compile | 19m 4s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 19m 4s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 46s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 35s | | the patch passed |
| +1 :green_heart: | xml | 0m 2s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 31s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 38s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 15m 14s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 38s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 23m 5s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 58s | | The patch does not generate ASF License warnings. |
| | | 198m 43s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux e0c847600b31 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 7b6cd12400cbb2bc8cf13dc126e2bb3e65693093 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/4/testReport/ |
| Max. process+thread count | 545 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/4/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-892470533
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 45s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| -1 :x: | mvninstall | 8m 47s | [/branch-mvninstall-root.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/8/artifact/out/branch-mvninstall-root.txt) | root in trunk failed. |
| +1 :green_heart: | compile | 2m 10s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 16s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 28s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 40s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 36s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 29s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 19s | | trunk passed |
| +1 :green_heart: | shadedclient | 14m 52s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 37s | | the patch passed |
| +1 :green_heart: | compile | 1m 21s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 1m 21s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 25s | | the patch passed |
| +1 :green_heart: | mvnsite | 0m 38s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 24s | | the patch passed |
| +1 :green_heart: | shadedclient | 14m 30s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 3s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 76m 24s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux e5746909aee5 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 822f72ca16f846f2bdfe4c07f31c03880ea86e93 |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/8/testReport/ |
| Max. process+thread count | 692 (vs. ulimit of 5500) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/8/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] shuzirra merged pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
shuzirra merged pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] 9uapaw commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
9uapaw commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-886690943
@tomicooler Thanks for the update! No additional comments on my side, +1 non-binding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] 9uapaw commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
9uapaw commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673800026
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
Review comment:
I think comments for the three parts (variable name matching, double quote matching, shell expansion matching) would be necessary for future reference. Perhaps an example would be helpful as well for each part.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ String variableName = matcher.group(VARIABLE_NAME);
+ if (variableName != null && !variableName.isEmpty()) {
+ deps.add(variableName);
+ }
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ fillDeps(deps, doubleQuoted);
+ }
+
+ String shellExpansion = matcher.group(SHELL_EXPANSION);
+ if (shellExpansion != null && !shellExpansion.isEmpty()) {
+ if (shellExpansion.charAt(0) == '#') {
+ shellExpansion = shellExpansion.substring(1);
}
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ String[] split = shellExpansion.split("[:#%/^,@\\[\\]]", 2);
+ if (split[0].length() > 0) {
+ deps.add(split[0]);
}
- c = envVal.charAt(i);
- if (c == '{') { // for ${... bash like syntax
- i++;
- if (i >= len) {
- break;
- }
- c = envVal.charAt(i);
- if (c == '#') { // for ${#... bash array syntax
- i++;
- if (i >= len) {
- break;
+ if (split.length == 2) {
+ Matcher variableMatcher = variableNamePattern.matcher(split[1]);
+ while (variableMatcher.find()) {
+ String vName = variableMatcher.group(VARIABLE_NAME);
Review comment:
These lines show resemblance of variable name matching. They might be brought together.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1547,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile("%(.*?)%");
Review comment:
I think this could be extracted as well.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
Review comment:
Nit: Win should be named bash.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_LINUX);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Stream<Arguments> inputForGetEnvDependenciesWin() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("%%%%%%", asSet()),
+ Arguments.of("%%A%", asSet()),
+ Arguments.of("%A", asSet()),
+ Arguments.of("%A:", asSet()),
+ Arguments.of("%A%", asSet("A")),
+ Arguments.of("%%%A%", asSet("A")),
+ Arguments.of("%%C%A%", asSet("A")),
+ Arguments.of("%A:~-1%", asSet("A")),
+ Arguments.of("%A:%", asSet("A")),
+ Arguments.of("%A:whatever:a:b:%", asSet("A")),
+ Arguments.of("%A%B%", asSet("A")),
+ Arguments.of("%A%%%%%B%", asSet("A")),
+ Arguments.of("%A%%B%", asSet("A", "B")),
+ Arguments.of("%A%%%%B%", asSet("A", "B")),
+ Arguments.of("%A%:%B%:%C%:pathlist var", asSet("A", "B", "C")),
+ Arguments.of("%A%\\\\foo\\\\bar:%B%:%C%:pathlist var",
+ asSet("A", "B", "C"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesWin")
+ void testGetEnvDependenciesWin(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
+ ContainerLaunch.ShellScriptBuilder.create(Shell.OSType.OS_TYPE_WIN);
+ Assert.assertEquals("Failed to parse " + input, expected,
+ win.getEnvDependencies(input));
+ }
+
+ private static Set<String> asSet(String... str) {
Review comment:
Nit: Guava has the same functionality as Sets.newHashSet.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ String variableName = matcher.group(VARIABLE_NAME);
+ if (variableName != null && !variableName.isEmpty()) {
+ deps.add(variableName);
+ }
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ fillDeps(deps, doubleQuoted);
+ }
+
+ String shellExpansion = matcher.group(SHELL_EXPANSION);
+ if (shellExpansion != null && !shellExpansion.isEmpty()) {
+ if (shellExpansion.charAt(0) == '#') {
+ shellExpansion = shellExpansion.substring(1);
}
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ String[] split = shellExpansion.split("[:#%/^,@\\[\\]]", 2);
Review comment:
I think this regex could be extracted to a static String like the others.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r674762493
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,64 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
Review comment:
Done, but checkstyle will fail on this line.
The @SuppressWarnings("checkstyle:linelength") could have been used, but it would have a bigger scope than just one line.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673917188
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1547,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile("%(.*?)%");
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673912213
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1410,53 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ String variableName = matcher.group(VARIABLE_NAME);
+ if (variableName != null && !variableName.isEmpty()) {
+ deps.add(variableName);
+ }
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ fillDeps(deps, doubleQuoted);
+ }
+
+ String shellExpansion = matcher.group(SHELL_EXPANSION);
+ if (shellExpansion != null && !shellExpansion.isEmpty()) {
+ if (shellExpansion.charAt(0) == '#') {
+ shellExpansion = shellExpansion.substring(1);
}
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ String[] split = shellExpansion.split("[:#%/^,@\\[\\]]", 2);
+ if (split[0].length() > 0) {
+ deps.add(split[0]);
}
- c = envVal.charAt(i);
- if (c == '{') { // for ${... bash like syntax
- i++;
- if (i >= len) {
- break;
- }
- c = envVal.charAt(i);
- if (c == '#') { // for ${#... bash array syntax
- i++;
- if (i >= len) {
- break;
+ if (split.length == 2) {
+ Matcher variableMatcher = variableNamePattern.matcher(split[1]);
+ while (variableMatcher.find()) {
+ String vName = variableMatcher.group(VARIABLE_NAME);
Review comment:
Extracted to a function.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-889861061
Force pushed the branch.
- Created a separate Jira for the unit test refactor (YARN-10874).
- Extracting the variable names from the bash version is not feasible with regexp, so that change is abandoned.
NOTE: the new regex batch variable name extraction works for the %:% variable, it will extract the : as the name. The original code did not find the : character to be a variable name. (https://stackoverflow.com/questions/37973141/colon-in-batch-variable-name/37992843). Since the test refactor is moved to separate pull request, I will add an extra test case with %:% to this patch if the other pull request is merged before this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r679887150
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1449,6 +1451,9 @@ public void setExitOnFailure() {
private static final class WindowsShellScriptBuilder
extends ShellScriptBuilder {
+ private static final String PATTERN_VARIABLE = "%(.*?)%";
+ private Pattern pattern;
Review comment:
Done. I also refactored the split by : to a pattern.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1544,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile(PATTERN_VARIABLE);
+ }
Review comment:
Done.
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1539,34 +1544,17 @@ public void listDebugInformation(Path output) throws IOException {
if (envVal == null || envVal.isEmpty()) {
return Collections.emptySet();
}
+ if (pattern == null) {
+ pattern = Pattern.compile(PATTERN_VARIABLE);
+ }
+ Matcher matcher = pattern.matcher(envVal);
final Set<String> deps = new HashSet<>();
- final int len = envVal.length();
- int i = 0;
- while (i < len) {
- i = envVal.indexOf('%', i); // find beginning of variable
- if (i < 0 || i == (len - 1)) {
- break;
- }
- i++;
- // 3 cases: %var%, %var:...% or %%
- final int j = envVal.indexOf('%', i); // find end of variable
- if (j == i) {
- // %% case, just skip it
- i++;
- continue;
- }
- if (j < 0) {
- break; // even %var:...% syntax ends with a %, so j cannot be negative
- }
- final int k = envVal.indexOf(':', i);
- if (k >= 0 && k < j) {
- // %var:...% syntax
- deps.add(envVal.substring(i, k));
- } else {
- // %var% syntax
- deps.add(envVal.substring(i, j));
+ while (matcher.find()) {
+ String match = matcher.group(1);
+ if (match.length() > 0) {
+ String[] split = match.split(":", 2);
+ deps.add(split[0]);
Review comment:
Done, added comments.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r674760283
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java
##########
@@ -1382,73 +1414,64 @@ public void setExitOnFailure() {
return Collections.emptySet();
}
final Set<String> deps = new HashSet<>();
- // env/whitelistedEnv dump values inside double quotes
- boolean inDoubleQuotes = true;
- char c;
- int i = 0;
- final int len = envVal.length();
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '"') {
- inDoubleQuotes = !inDoubleQuotes;
- } else if (c == '\'' && !inDoubleQuotes) {
- i++;
- // eat until closing simple quote
- while (i < len) {
- c = envVal.charAt(i);
- if (c == '\\') {
- i++;
- }
- if (c == '\'') {
- break;
- }
- i++;
- }
- } else if (c == '\\') {
- i++;
- } else if (c == '$') {
- i++;
- if (i >= len) {
- break;
+ fillDeps(deps, envVal);
+ return deps;
+ }
+
+ private void fillDeps(final Set<String> deps, final String input) {
+ Matcher matcher = mainPattern.matcher(input);
+ while (matcher.find()) {
+ // simple $variableName extraction
+ extractVariableNameToDeps(deps, matcher);
+
+ String doubleQuoted = matcher.group(DOUBLE_QUOTED);
+ if (doubleQuoted != null) {
+ // env/whitelistedEnv dump values inside double quotes
+ // single quotes inside double quotes are ignored
+ doubleQuoted =
+ singleQuotePattern.matcher(doubleQuoted).replaceAll("");
+ // the remaining is processed again
+ fillDeps(deps, doubleQuoted);
+ }
+
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ // matching shell expansion variables, e.g.:
+ // ${variableName} ${foo:-$bar} ${#bar} etc
Review comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-889909629
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 12m 57s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| -1 :x: | test4tests | 0m 0s | | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 31m 26s | | trunk passed |
| +1 :green_heart: | compile | 1m 35s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 1m 23s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 0m 34s | | trunk passed |
| +1 :green_heart: | mvnsite | 0m 48s | | trunk passed |
| +1 :green_heart: | javadoc | 0m 41s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 33s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 27s | | trunk passed |
| +1 :green_heart: | shadedclient | 14m 34s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 0m 39s | | the patch passed |
| +1 :green_heart: | compile | 1m 22s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 1m 22s | | the patch passed |
| +1 :green_heart: | compile | 1m 16s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 1m 16s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| -0 :warning: | checkstyle | 0m 24s | [/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/7/artifact/out/results-checkstyle-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-nodemanager.txt) | hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager: The patch generated 2 new + 35 unchanged - 0 fixed = 37 total (was 35) |
| +1 :green_heart: | mvnsite | 0m 39s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 30s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 0m 29s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | spotbugs | 1m 28s | | the patch passed |
| +1 :green_heart: | shadedclient | 14m 33s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 23m 1s | | hadoop-yarn-server-nodemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 34s | | The patch does not generate ASF License warnings. |
| | | 111m 10s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell |
| uname | Linux 726052fe5100 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f7a369f7e97a2435026d26769ce87a80d84c1b2c |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/7/testReport/ |
| Max. process+thread count | 676 (vs. ulimit of 5500) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/7/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] hadoop-yetus commented on pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
hadoop-yetus commented on pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#issuecomment-883589579
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 0m 55s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +0 :ok: | mvndep | 13m 17s | | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 20m 6s | | trunk passed |
| +1 :green_heart: | compile | 21m 14s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | compile | 18m 18s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | checkstyle | 3m 46s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 43s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 31s | | trunk passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 28s | | trunk passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 42s | | branch/hadoop-project no spotbugs output file (spotbugsXml.xml) |
| +1 :green_heart: | shadedclient | 14m 59s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 32s | | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 0m 52s | | the patch passed |
| +1 :green_heart: | compile | 20m 29s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javac | 20m 29s | | the patch passed |
| +1 :green_heart: | compile | 18m 26s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +1 :green_heart: | javac | 18m 26s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 3m 47s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 40s | | the patch passed |
| +1 :green_heart: | xml | 0m 3s | | The patch has no ill-formed XML file. |
| +1 :green_heart: | javadoc | 1m 31s | | the patch passed with JDK Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 |
| +1 :green_heart: | javadoc | 1m 28s | | the patch passed with JDK Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| +0 :ok: | spotbugs | 0m 39s | | hadoop-project has no data from spotbugs |
| +1 :green_heart: | shadedclient | 15m 13s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 0m 37s | | hadoop-project in the patch passed. |
| +1 :green_heart: | unit | 23m 16s | | hadoop-yarn-server-nodemanager in the patch passed. |
| -1 :x: | asflicense | 0m 59s | [/results-asflicense.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/1/artifact/out/results-asflicense.txt) | The patch generated 1 ASF License warnings. |
| | | 193m 44s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/3220 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell xml spotbugs checkstyle |
| uname | Linux f7fd94d7fb65 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / fc5bf7cab270c39d4a5361f045633e71417c3dcf |
| Default Java | Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Multi-JDK versions | /usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.11+9-Ubuntu-0ubuntu2.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_292-8u292-b10-0ubuntu1~20.04-b10 |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/1/testReport/ |
| Max. process+thread count | 548 (vs. ulimit of 5500) |
| modules | C: hadoop-project hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager U: . |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-3220/1/console |
| versions | git=2.25.1 maven=3.6.3 spotbugs=4.2.2 |
| Powered by | Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org |
This message was automatically generated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
[GitHub] [hadoop] tomicooler commented on a change in pull request #3220: YARN-10355. Refactor NM ContainerLaunch.java#orderEnvByDependencies
Posted by GitBox <gi...@apache.org>.
tomicooler commented on a change in pull request #3220:
URL: https://github.com/apache/hadoop/pull/3220#discussion_r673917304
##########
File path: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/TestContainerLaunchParameterized.java
##########
@@ -0,0 +1,148 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.yarn.server.nodemanager.containermanager.launcher;
+
+import org.apache.hadoop.util.Shell;
+import org.junit.Assert;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.stream.Stream;
+
+public class TestContainerLaunchParameterized {
+ private static Stream<Arguments> inputForGetEnvDependenciesLinux() {
+ return Stream.of(
+ Arguments.of(null, asSet()),
+ Arguments.of("", asSet()),
+ Arguments.of("A", asSet()),
+ Arguments.of("\\$A", asSet()),
+ Arguments.of("$$", asSet()),
+ Arguments.of("$1", asSet()),
+ Arguments.of("handle \"'$A'\" simple quotes", asSet()),
+ Arguments.of("handle \" escaped \\\" '${A}'\" simple quotes", asSet()),
+ Arguments
+ .of("$ crash test for StringArrayOutOfBoundException", asSet()),
+ Arguments.of("${ crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments.of("${# crash test for StringArrayOutOfBoundException",
+ asSet()),
+ Arguments
+ .of("crash test for StringArrayOutOfBoundException $", asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${",
+ asSet()),
+ Arguments.of("crash test for StringArrayOutOfBoundException ${#",
+ asSet()),
+ Arguments.of("$A", asSet("A")),
+ Arguments.of("${A}", asSet("A")),
+ Arguments.of("${#A[*]}", asSet("A")),
+ Arguments.of("in the $A midlle", asSet("A")),
+ Arguments.of("${A:-$B} var in var", asSet("A", "B")),
+ Arguments.of("${A}$B var outside var", asSet("A", "B")),
+ Arguments.of("$A:$B:$C:pathlist var", asSet("A", "B", "C")),
+ Arguments
+ .of("${A}/foo/bar:$B:${C}:pathlist var", asSet("A", "B", "C")),
+ // https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter
+ // -Expansion.html
+ Arguments.of("${parameter:-word}", asSet("parameter")),
+ Arguments.of("${parameter:=word}", asSet("parameter")),
+ Arguments.of("${parameter:?word}", asSet("parameter")),
+ Arguments.of("${parameter:+word}", asSet("parameter")),
+ Arguments.of("${parameter:71}", asSet("parameter")),
+ Arguments.of("${parameter:71:30}", asSet("parameter")),
+ Arguments.of("!{prefix*}", asSet()),
+ Arguments.of("${!prefix@}", asSet()),
+ Arguments.of("${!name[@]}", asSet()),
+ Arguments.of("${!name[*]}", asSet()),
+ Arguments.of("${#parameter}", asSet("parameter")),
+ Arguments.of("${parameter#word}", asSet("parameter")),
+ Arguments.of("${parameter##word}", asSet("parameter")),
+ Arguments.of("${parameter%word}", asSet("parameter")),
+ Arguments.of("${parameter/pattern/string}", asSet("parameter")),
+ Arguments.of("${parameter^pattern}", asSet("parameter")),
+ Arguments.of("${parameter^^pattern}", asSet("parameter")),
+ Arguments.of("${parameter,pattern}", asSet("parameter")),
+ Arguments.of("${parameter,,pattern}", asSet("parameter")),
+ Arguments.of("${parameter@o}", asSet("parameter")),
+ Arguments.of("handle '${A}' simple quotes", asSet("A")),
+ Arguments.of("handle '${A} $B ${C:-$D}' simple quotes",
+ asSet("A", "B", "C", "D")),
+ Arguments.of("handle \"'${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle \"'\\${A}'\" double and single quotes", asSet()),
+ Arguments.of("handle '\\${A} \\$B \\${C:-D}' single quotes", asSet()),
+ Arguments.of("handle \"${A}\" double quotes", asSet("A")),
+ Arguments.of("handle \"${A} $B ${C:-$D}\" double quotes",
+ asSet("A", "B", "C", "D"))
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("inputForGetEnvDependenciesLinux")
+ void testGetEnvDependenciesLinux(String input,
+ Set<String> expected) {
+ ContainerLaunch.ShellScriptBuilder win =
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org