Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support comments in jvm-opts file #66

Closed
fernandoacorreia opened this issue Jan 19, 2014 · 9 comments · Fixed by mohiva/play-silhouette#126
Closed

Support comments in jvm-opts file #66

fernandoacorreia opened this issue Jan 19, 2014 · 9 comments · Fixed by mohiva/play-silhouette#126
Labels

Comments

@fernandoacorreia
Copy link

When using a jvm-opts file with comments like this:

# Comment 1
# Comment 2

-Xms2048M
-Xmx2048M
-Xss6M
-XX:MaxPermSize=512M

and a command line like this:

sbt -v -jvm-opts conf/jvm-opts clean coveralls doc

this error happens:

No extra sbt options have been defined
Detected sbt version 0.13.0
Using /home/fernando/.sbt/0.13.0 as sbt dir, -sbt-dir to override.
Using jvm options defined in file scripts/conf/jvm-opts
# Executing command line:
/usr/lib/jvm/java-7-oracle/bin/java
"# Comment 1"
"# Comment 2"
-Xms2048M
-Xmx2048M
-Xss6M
-XX:MaxPermSize=512M
-jar
/home/fernando/.sbt/launchers/0.13.0/sbt-launch.jar
clean
coveralls
doc

Error: Could not find or load main class # Comment 1

This situation will also happen when building on Travis CI, which uses this configuration:

# Tweaking for VM equipped with 3GB of RAM
# See also 'default_jvm_opts' defined in <%= File.join(node['sbt-extras']['setup_dir'], node['sbt-extras']['script_name']) %>

-Xms2048M
-Xmx2048M
-Xss6M
-XX:MaxPermSize=512M

For an example see this failed Travis CI build triggered by this script.

@paulp
Copy link
Collaborator

paulp commented Feb 24, 2014

Amazingly this has never worked in the year since it was checked in! A resounding statement that untested projects are going to be big disappointments. Thankfully @yyuu did something about the tests and since the windows aren't so broken anymore I fixed and tested this in 73aba99 .

@gildegoma
Copy link
Contributor

I'd like to clarify a few more things about this bug report:

This situation will also happen when building on Travis CI...

No, the version of sbt-extras (2fd0642) that currently runs on Travis CI is not (so) flawed as reported here. By default /etc/sbt/jvmopts is successfully loaded (skipping the two comments and the blank line mentioned above), as illustrated by these examples:

For an example see this failed Travis CI build triggered by this script.

In mohiva/play-silhouette@bd17777, Travis script was customized to run with a more recent version of sbt-extras (apparently b170cdd), which indeed is broken.

I didn't take time to identify the cause of the regression... But yeah, having automated tests/specs is the only way!

As still unexperienced shellscripter and moreover non-native english speaker, I am not certain to fully understand last part of 73aba99 commit comment:

, but lines with a non-initial # are options

@paulp Do you want to support non-initial # comments or not?
I ask because my original regexp-based implementation supported it...

PS: This is great that sbt-extras is now checked by travis :)

@paulp
Copy link
Collaborator

paulp commented Feb 24, 2014

"I ask because my original regexp-based implementation supported it..."

Did it handle all the forms of quoting which might appear? The problem is if you want to pass a string containing a #, e.g. as part of a system property. Even if it did handle this - and it's really tricky with regexps - I find the feature unnecessary and prefer the implementation which doesn't take on baroque quoting issues.

@paulp
Copy link
Collaborator

paulp commented Feb 24, 2014

@gildegoma Indeed try your runner with a .jvmopts of

-Dfoo.bar="my #string"

and you will see what I mean.

@gildegoma
Copy link
Contributor

Sure, I understand :)

"I ask because my original regexp-based implementation supported it..."

You're right to correct me on this point. Sorry for writing this line too fast. I was not pretending my implementation was so robust, but I just wanted to be sure that we all agree to drop this feature.

And fixing this weakness is actually a good argument to update the sbt-extras version on Travis CI as well. I'll take care of this...

@paulp
Copy link
Collaborator

paulp commented Feb 24, 2014

It wasn't exactly fixed until just now, but as of 4f44188 I believe it is.

@gildegoma
Copy link
Contributor

@paulp looks good! Thanks again for all your amazing work!
I'll keep you informed about the update on Travis CI.

@fernandoacorreia
Copy link
Author

Thank you @paulp and @gildegoma for looking into this.

@gildegoma
Copy link
Contributor

FYI: the JVM worker machine on Travis CI (.org) will be updated in the next days with 60b6f26. The exact rollout date will be announced on http://blog.travis-ci.com.

Note that sbt 0.13.2 won't be pre-installed in this VM image, but in next VM update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants