You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2019/12/28 21:47:04 UTC
[GitHub] [netbeans] lkishalmi opened a new pull request #1827:
[NETBEANS-3041] Honor gradle.properties processing order for JVM args.
lkishalmi opened a new pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi merged pull request #1827: [NETBEANS-3041]
Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
lkishalmi merged pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a change in pull request
#1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361834584
##########
File path: groovy/gradle/test/unit/src/org/netbeans/modules/gradle/api/execute/GradleCommandLineTest.java
##########
@@ -277,4 +284,26 @@ public void testCombine3() {
GradleCommandLine cmd = GradleCommandLine.combine(first, second);
assertEquals(Arrays.asList("-Pversion=2.0", "build"), cmd.getSupportedCommandLine());
}
+
+ @Test
+ public void testJVMArgs1() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Xmx2g"));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Xmx2g"), jvmargs);
+ }
+
+ @Test
+ public void testJVMArgs2() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Dsomething=\"space value\""));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Dsomething=space value"), jvmargs);
Review comment:
This looks suspicious. I would have expected:
```java
assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Dsomething=\"space value\""), jvmargs);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on a change in pull request #1827:
[NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361863312
##########
File path: groovy/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -825,45 +826,56 @@ public void setStackTrace(StackTrace st) {
}
}
- private void addGradleSettingJvmargs(File projectDir, List<String> jvmargs) {
+ final static void addGradleSettingJvmargs(File rootDir, List<String> jvmargs) {
List<File> propFiles = new ArrayList<>();
+ propFiles.add(new File(GradleSettings.getDefault().getGradleUserHome(), GradleFiles.GRADLE_PROPERTIES_NAME));
- if (projectDir == null) {
- File gradleHome = GradleSettings.getDefault().getGradleUserHome();
- File f = new File(gradleHome, GradleFiles.GRADLE_PROPERTIES_NAME);
- if (f.exists()) {
- propFiles.add(f);
- }
- } else {
- propFiles.addAll(new GradleFiles(projectDir).getPropertyFiles());
+ if (rootDir != null) {
+ propFiles.addAll(new GradleFiles(rootDir).getPropertyFiles());
}
-
+ //TODO: Theoretically the Gradle Distribution dir can have a gradle.properties
+ // however computing that is not really easy, at the moment we do
+ // not support that one.
for (File f : propFiles) {
- try (InputStream in = new FileInputStream(f)) {
- Properties props = new Properties();
- props.load(in);
- if (props.containsKey(PROP_JVMARGS)) {
- jvmargs.addAll(Arrays.asList(props.getProperty(PROP_JVMARGS).split("\\s+"))); //NOI18N
+ if (f.canRead()) {
+ try (InputStream in = new FileInputStream(f)) {
+ Properties props = new Properties();
+ props.load(in);
+ if (props.containsKey(PROP_JVMARGS)) {
+ String[] args = Utilities.parseParameters(props.getProperty(PROP_JVMARGS));
+ for (String arg : args) {
+ jvmargs.add(removeSingleQuotes(arg));
+ }
+ break;
+ }
+ } catch (IOException ex) {
+ LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
- } catch (IOException ex) {
- LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
}
}
+ final static String removeSingleQuotes(String s) {
Review comment:
Well thought it over and fixed the BaseUtilities.parseParameters instead in #1829 after all it was fun.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on a change in pull request #1827:
[NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361854556
##########
File path: groovy/gradle/test/unit/src/org/netbeans/modules/gradle/api/execute/GradleCommandLineTest.java
##########
@@ -277,4 +284,26 @@ public void testCombine3() {
GradleCommandLine cmd = GradleCommandLine.combine(first, second);
assertEquals(Arrays.asList("-Pversion=2.0", "build"), cmd.getSupportedCommandLine());
}
+
+ @Test
+ public void testJVMArgs1() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Xmx2g"));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Xmx2g"), jvmargs);
+ }
+
+ @Test
+ public void testJVMArgs2() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Dsomething=\"space value\""));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Dsomething=space value"), jvmargs);
Review comment:
It should be really "-Dsomething=space value" as that would be passed to JVM and the JVM does parse the quotes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a change in pull request
#1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361856518
##########
File path: groovy/gradle/test/unit/src/org/netbeans/modules/gradle/api/execute/GradleCommandLineTest.java
##########
@@ -277,4 +284,26 @@ public void testCombine3() {
GradleCommandLine cmd = GradleCommandLine.combine(first, second);
assertEquals(Arrays.asList("-Pversion=2.0", "build"), cmd.getSupportedCommandLine());
}
+
+ @Test
+ public void testJVMArgs1() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Xmx2g"));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Xmx2g"), jvmargs);
+ }
+
+ @Test
+ public void testJVMArgs2() throws IOException {
+ TemporaryFolder root = new TemporaryFolder();
+ root.create();
+ File props = root.newFile("gradle.properties");
+ Files.write(props.toPath(), Arrays.asList("org.gradle.jvmargs=\"-Dfile.encoding=UTF-8\" -Dsomething=\"space value\""));
+ List<String> jvmargs = new ArrayList<>();
+ GradleCommandLine.addGradleSettingJvmargs(root.getRoot(), jvmargs);
+ assertEquals(Arrays.asList("-Dfile.encoding=UTF-8", "-Dsomething=space value"), jvmargs);
Review comment:
Ok - verified with a small java programm.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a change in pull request
#1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361834628
##########
File path: groovy/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -825,45 +826,56 @@ public void setStackTrace(StackTrace st) {
}
}
- private void addGradleSettingJvmargs(File projectDir, List<String> jvmargs) {
+ final static void addGradleSettingJvmargs(File rootDir, List<String> jvmargs) {
List<File> propFiles = new ArrayList<>();
+ propFiles.add(new File(GradleSettings.getDefault().getGradleUserHome(), GradleFiles.GRADLE_PROPERTIES_NAME));
- if (projectDir == null) {
- File gradleHome = GradleSettings.getDefault().getGradleUserHome();
- File f = new File(gradleHome, GradleFiles.GRADLE_PROPERTIES_NAME);
- if (f.exists()) {
- propFiles.add(f);
- }
- } else {
- propFiles.addAll(new GradleFiles(projectDir).getPropertyFiles());
+ if (rootDir != null) {
+ propFiles.addAll(new GradleFiles(rootDir).getPropertyFiles());
}
-
+ //TODO: Theoretically the Gradle Distribution dir can have a gradle.properties
+ // however computing that is not really easy, at the moment we do
+ // not support that one.
for (File f : propFiles) {
- try (InputStream in = new FileInputStream(f)) {
- Properties props = new Properties();
- props.load(in);
- if (props.containsKey(PROP_JVMARGS)) {
- jvmargs.addAll(Arrays.asList(props.getProperty(PROP_JVMARGS).split("\\s+"))); //NOI18N
+ if (f.canRead()) {
+ try (InputStream in = new FileInputStream(f)) {
+ Properties props = new Properties();
+ props.load(in);
+ if (props.containsKey(PROP_JVMARGS)) {
+ String[] args = Utilities.parseParameters(props.getProperty(PROP_JVMARGS));
+ for (String arg : args) {
+ jvmargs.add(removeSingleQuotes(arg));
+ }
+ break;
+ }
+ } catch (IOException ex) {
+ LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
- } catch (IOException ex) {
- LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
}
}
+ final static String removeSingleQuotes(String s) {
Review comment:
What is the reason for the special handling of single quotes after they are already parsed?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] lkishalmi commented on a change in pull request #1827:
[NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
lkishalmi commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361854722
##########
File path: groovy/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -825,45 +826,56 @@ public void setStackTrace(StackTrace st) {
}
}
- private void addGradleSettingJvmargs(File projectDir, List<String> jvmargs) {
+ final static void addGradleSettingJvmargs(File rootDir, List<String> jvmargs) {
List<File> propFiles = new ArrayList<>();
+ propFiles.add(new File(GradleSettings.getDefault().getGradleUserHome(), GradleFiles.GRADLE_PROPERTIES_NAME));
- if (projectDir == null) {
- File gradleHome = GradleSettings.getDefault().getGradleUserHome();
- File f = new File(gradleHome, GradleFiles.GRADLE_PROPERTIES_NAME);
- if (f.exists()) {
- propFiles.add(f);
- }
- } else {
- propFiles.addAll(new GradleFiles(projectDir).getPropertyFiles());
+ if (rootDir != null) {
+ propFiles.addAll(new GradleFiles(rootDir).getPropertyFiles());
}
-
+ //TODO: Theoretically the Gradle Distribution dir can have a gradle.properties
+ // however computing that is not really easy, at the moment we do
+ // not support that one.
for (File f : propFiles) {
- try (InputStream in = new FileInputStream(f)) {
- Properties props = new Properties();
- props.load(in);
- if (props.containsKey(PROP_JVMARGS)) {
- jvmargs.addAll(Arrays.asList(props.getProperty(PROP_JVMARGS).split("\\s+"))); //NOI18N
+ if (f.canRead()) {
+ try (InputStream in = new FileInputStream(f)) {
+ Properties props = new Properties();
+ props.load(in);
+ if (props.containsKey(PROP_JVMARGS)) {
+ String[] args = Utilities.parseParameters(props.getProperty(PROP_JVMARGS));
+ for (String arg : args) {
+ jvmargs.add(removeSingleQuotes(arg));
+ }
+ break;
+ }
+ } catch (IOException ex) {
+ LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
- } catch (IOException ex) {
- LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
}
}
+ final static String removeSingleQuotes(String s) {
Review comment:
Unfortunately the command line parser in NetBeans does not parse the single quotes. It's maybe a bug there. It was easier to add a workaround to the most common case here than fix it there. Also the impact of fixing it in the utilities might have unwanted consequences.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
[GitHub] [netbeans] matthiasblaesing commented on a change in pull request
#1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
Posted by GitBox <gi...@apache.org>.
matthiasblaesing commented on a change in pull request #1827: [NETBEANS-3041] Honor gradle.properties processing order for JVM args.
URL: https://github.com/apache/netbeans/pull/1827#discussion_r361857539
##########
File path: groovy/gradle/src/org/netbeans/modules/gradle/api/execute/GradleCommandLine.java
##########
@@ -825,45 +826,56 @@ public void setStackTrace(StackTrace st) {
}
}
- private void addGradleSettingJvmargs(File projectDir, List<String> jvmargs) {
+ final static void addGradleSettingJvmargs(File rootDir, List<String> jvmargs) {
List<File> propFiles = new ArrayList<>();
+ propFiles.add(new File(GradleSettings.getDefault().getGradleUserHome(), GradleFiles.GRADLE_PROPERTIES_NAME));
- if (projectDir == null) {
- File gradleHome = GradleSettings.getDefault().getGradleUserHome();
- File f = new File(gradleHome, GradleFiles.GRADLE_PROPERTIES_NAME);
- if (f.exists()) {
- propFiles.add(f);
- }
- } else {
- propFiles.addAll(new GradleFiles(projectDir).getPropertyFiles());
+ if (rootDir != null) {
+ propFiles.addAll(new GradleFiles(rootDir).getPropertyFiles());
}
-
+ //TODO: Theoretically the Gradle Distribution dir can have a gradle.properties
+ // however computing that is not really easy, at the moment we do
+ // not support that one.
for (File f : propFiles) {
- try (InputStream in = new FileInputStream(f)) {
- Properties props = new Properties();
- props.load(in);
- if (props.containsKey(PROP_JVMARGS)) {
- jvmargs.addAll(Arrays.asList(props.getProperty(PROP_JVMARGS).split("\\s+"))); //NOI18N
+ if (f.canRead()) {
+ try (InputStream in = new FileInputStream(f)) {
+ Properties props = new Properties();
+ props.load(in);
+ if (props.containsKey(PROP_JVMARGS)) {
+ String[] args = Utilities.parseParameters(props.getProperty(PROP_JVMARGS));
+ for (String arg : args) {
+ jvmargs.add(removeSingleQuotes(arg));
+ }
+ break;
+ }
+ } catch (IOException ex) {
+ LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
- } catch (IOException ex) {
- LOGGER.log(Level.INFO, "Cannot read property file: '" + f.getAbsolutePath() + "' as: " + ex.getMessage());
}
}
}
+ final static String removeSingleQuotes(String s) {
Review comment:
But this only works for the simplest of cases doesn't it? This:
```java
import java.util.Arrays;
class test {
public static void main(String[] argv) {
System.out.println(Arrays.toString(argv));
}
}
```
Run as:
```
matthias@athena:~$ java -cp . test -Dsomething="space value"
[-Dsomething=space value]
matthias@athena:~$ java -cp . test -Dsomething='space value'
[-Dsomething=space value]
matthias@athena:~$
```
shows, that also embedded single quotes need to be removed. Is this really needed or just a theoretical issue?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists