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

glassfish 5.1.0 #48240

Merged
merged 1 commit into from Jan 5, 2020
Merged

glassfish 5.1.0 #48240

merged 1 commit into from Jan 5, 2020

Conversation

@joaomlneto
Copy link
Contributor

joaomlneto commented Dec 23, 2019

  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

The test is fairly minimal, but I have no idea how to do them :-)
I could try to do something similar to the wildfly-as formula


Glassfish has been transfered to the Eclipse Foundation from version 5.1

Currently there is no proper homepage. Candidates:

@chenrui333 chenrui333 changed the title Update Glassfish to 5.1 from Eclipse glassfish: 5.1.0 Dec 24, 2019
@chenrui333 chenrui333 changed the title glassfish: 5.1.0 glassfish 5.1.0 Dec 24, 2019
@SMillerDev

This comment has been minimized.

Copy link
Member

SMillerDev commented Dec 24, 2019

This test isn't doing much so please try your suggestion of copying the other test.

Just use the github page as homepage.

Formula/glassfish.rb Outdated Show resolved Hide resolved
@joaomlneto

This comment has been minimized.

Copy link
Contributor Author

joaomlneto commented Dec 24, 2019

@SMillerDev I have addressed all your comments.

One potential issue with the testing is that Glassfish requires Java 8 to start (exactly version 8). Should I change the depends_on attribute? I have changed it to match payara.rb, which is a fork of GlassFish.

@fxcoudert

This comment has been minimized.

Copy link
Member

fxcoudert commented Dec 26, 2019

==> brew test glassfish --verbose
==> FAILED
Testing glassfish
/usr/bin/sandbox-exec -f /private/tmp/homebrew20191226-75747-1mrwsex.sb ruby -W0 -I $LOAD_PATH -- /usr/local/Homebrew/Library/Homebrew/test.rb /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/glassfish.rb --verbose
==> /usr/local/opt/glassfish/libexec/glassfish/bin/asadmin version | grep 5.1.0
Picked up _JAVA_OPTIONS: -Duser.home=/Users/brew/Library/Caches/Homebrew/java_cache
Version = GlassFish Server Open Source Edition  5.1.0  (build default-private)
Picked up _JAVA_OPTIONS: -Duser.home=/Users/brew/Library/Caches/Homebrew/java_cache
Exception in thread "main" java.lang.NullPointerException
	at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.initializeServiceLocator(AbstractModulesRegistryImpl.java:128)
	at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.newServiceLocator(AbstractModulesRegistryImpl.java:120)
	at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.createServiceLocator(AbstractModulesRegistryImpl.java:194)
	at com.sun.enterprise.module.common_impl.AbstractModulesRegistryImpl.createServiceLocator(AbstractModulesRegistryImpl.java:200)
	at com.sun.enterprise.module.single.StaticModulesRegistry.createServiceLocator(StaticModulesRegistry.java:64)
	at com.sun.enterprise.admin.cli.CLIContainer.getServiceLocator(CLIContainer.java:193)
	at com.sun.enterprise.admin.cli.CLIContainer.getLocalCommand(CLIContainer.java:231)
	at com.sun.enterprise.admin.cli.CLICommand.getCommand(CLICommand.java:207)
	at com.sun.enterprise.admin.cli.AdminMain.executeCommand(AdminMain.java:347)
	at com.sun.enterprise.admin.cli.AdminMain.doMain(AdminMain.java:282)
	at org.glassfish.admin.cli.AsadminMain.main(AsadminMain.java:33)
@joaomlneto

This comment has been minimized.

Copy link
Contributor Author

joaomlneto commented Dec 26, 2019

@fxcoudert: That is most likely due to it being executed with a incompatible Java version[1] -- it works exclusively with Java 8.

I'm unsure how to encode this in the Formula. Any ideas?

This behavior was happening with the previous version of Glassfish.

@fxcoudert fxcoudert added the java label Dec 26, 2019
@joaomlneto

This comment has been minimized.

Copy link
Contributor Author

joaomlneto commented Dec 30, 2019

I am now telling GlassFish to use JDK8 during the tests.

However, I'm doing this /usr/libexec/java_home, which seems to be MacOS-only (and not even sure if it is the best way of doing it).

Is there any HomeBrew recommendation on getting JAVA_HOME for a specific JDK?

@fxcoudert

This comment has been minimized.

Copy link
Member

fxcoudert commented Jan 3, 2020

We have Language::Java.java_home_cmd("1.8"), see how it's used in other formulas

@joaomlneto joaomlneto requested a review from SMillerDev Jan 4, 2020
Copy link
Member

chenrui333 left a comment

lgtm

@zbeekman

This comment has been minimized.

Copy link
Member

zbeekman commented Jan 4, 2020

@joaomlneto any chance you could squash these commits and force push?

Updates GlassFish to version 5.1, now living at the Eclipse Foundation (previously Oracle).
Adds a test to the GlassFish formula
@joaomlneto joaomlneto force-pushed the joaomlneto:eclipse-glassfish-5.1 branch from 416ea94 to 154e0c3 Jan 5, 2020
@joaomlneto

This comment has been minimized.

Copy link
Contributor Author

joaomlneto commented Jan 5, 2020

@zbeekman done :-)


many thanks for the Homebrew project!
and thanks for putting up with my questions here!
@chenrui333

This comment has been minimized.

Copy link
Member

chenrui333 commented Jan 5, 2020

Thanks @joaomlneto for your contribution to Homebrew! 🎉 🥇

Without awesome contributors like you, it would be impossible to maintain Homebrew to the high level of quality users have come to expect. Thank you!!!!

@chenrui333 chenrui333 merged commit e9a1fa8 into Homebrew:master Jan 5, 2020
1 check passed
1 check passed
continuous-integration/jenkins/ghprb Build finished.
Details
@joaomlneto joaomlneto deleted the joaomlneto:eclipse-glassfish-5.1 branch Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.