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

bring generator to homebrew #26

Closed
jothepro opened this issue Jan 21, 2021 · 13 comments · Fixed by #72
Closed

bring generator to homebrew #26

jothepro opened this issue Jan 21, 2021 · 13 comments · Fixed by #72
Labels
idea An idea that needs further discussion/refinement

Comments

@jothepro
Copy link
Contributor

As a mac user I think it would be super cool if I could just install the djinni-generator from homebrew.

@jothepro jothepro added the idea An idea that needs further discussion/refinement label Jan 21, 2021
@a4z
Copy link
Contributor

a4z commented Jan 22, 2021

I agree! But there is the java dependency that could make things complicated
Lets find out how that can be sorted out

@joprice
Copy link
Contributor

joprice commented Jan 31, 2021

Graal? https://github.com/cross-language-cpp/djinni-generator/compare/main...joprice:graal?expand=1. I can pr that and work to finish integrate it if you're interested.

Also, it takes 1ms to start and print the Error: Missing option --idl when provided no args, compared to the 1s or more for the jvm version.

@a4z
Copy link
Contributor

a4z commented Jan 31, 2021

Thanks you for that info, Joseph, this is an interesting observation!
I should give a graal jvm trial, for generating source code this is possible less relevant, since this runs not very often, but for development that could be nice.

Afaik the generator picks up the jvm found first, this can maybe be Graal, or something different, depending on how your system is configured.
Adding a hard dependency for graal is probably not the most user-friendly alternative, we should let user choose what they want to have on the system.

In the context of this ticket, providing the generator for homebrew, I guess openjdk dependency would be a sane default, and maybe this can be tweaked through options.

brew search jdk
==> Formulae
openjdk ✔                  openjdk@11                 openjdk@8
==> Casks
adoptopenjdk               oracle-jdk                 sapmachine-jdk
jdk-mission-control        oracle-jdk-javadoc

@jothepro
Copy link
Contributor Author

jothepro commented Jan 31, 2021

@a4z Afaik the example that @joprice gave does compile to native binaries, no vm or any other dependency needed at runtime.

I have tried to compile the generator with graalvm a while ago, but I could not make it work.
@joprice looking forward to your PR, this was on my wishlist for some time now!

@joprice
Copy link
Contributor

joprice commented Jan 31, 2021

@a4z Graal is a big project. I'm referring in this case to native-image, which can compile jvm bytecode to machine code. The final binary can still be quite large, but it removes the jvm dependency and makes startup and runtime much faster.

In my case, I wanted this because I was iterating on bazel rules that wrap djinni and when running the generating hundreds of times, one second vs .001 seconds starts to feel noticeable.

For the initial pr, I could just put up what I have so that it's on master and easy to try out, then iterate on a few things:

@joprice
Copy link
Contributor

joprice commented Jan 31, 2021

I split out the sbt update here #29. This makes it much more pleasant at least for me to iterate.

@joprice
Copy link
Contributor

joprice commented Jan 31, 2021

I could also help set up sbt-native-packager and use that to get brew installation going. Then, once the graal version works, it's easy to switch to it for the extra benefits. It can also exist as a tap to avoid having to pr to homebrew until it's ready. Here's an example https://github.com/joprice/homebrew-tap/blob/master/dynamite.rb.

@jothepro
Copy link
Contributor Author

Do you intend to support macos and linux?

the generator should support macos, linux, windows. It would be super cool to have native binaries for all these platforms.

looks like there's a mac runnner here

Yep, @a4z has already used the mac runner successfully to build the support lib.

I've used sbt-github-release before to do releases

I don't think this works with our current release process. The current approach is to manually create a release on github, which will then trigger the build.

Then, once the graal version works, it's easy to switch to it for the extra benefits.

I agree that the graal binaries should be an alternative to the current build to begin with.

@a4z
Copy link
Contributor

a4z commented Jan 31, 2021

If Graal is just a build dependency, and it skips the JVM runtime dependency, this is good.
Does it create 1 binary, with no dynamic libraries needed from the Graal package?

Please make it optional. Maybe with the current behaviour as default (for the begin, we can switch later to it as default). Since this seems to remove a runtime dependency (is this really true?), we can, and should, base our distribution on it, if this is OK with the Graal license.

But there are people that insist of building software from source to integrate it, and we do not want to make the live for them harder as it needs to be. We also do not want to break existing workflows over night. Graal is huge, openjdk mostly already available, also maybe on more platforms.
So the current way of doing it should not be removed over night.

But I am very exciting learning about Graal and using it!

@joprice
Copy link
Contributor

joprice commented Jan 31, 2021

It creates one binary including all direct dependencies plus whatever parts of the jvm runtime are used - gc, etc. It can optionally be statically linked using musl, or libc https://www.graalvm.org/reference-manual/native-image/StaticImages/.

Apple doesn't like static linking, so there are a few standard frameworks linked by default:

djinni-generator git:(graal) ✗ otool -L target/graalvm-native-image/djinni
target/graalvm-native-image/djinni:
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.60.1)
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1122.11.0)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 904.4.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1770.255.0)
	/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1770.255.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)

The behavior would definitely be optional. It would be a separate binary that would be added as an asset to the release next to the existing fat executable jar, and would not affect building from source or change any workflow with the existing sbt build, only add one extra command if you want to build the native image.

The license is GPLv2+CPE https://github.com/oracle/graal/blob/master/LICENSE. I'm not qualified to give legal advice, but as far as I can tell, the Classpath Exception allows for static or dynamic linking without forcing the project to be GPL oracle/graal#2025 (comment).

The main downside I didn't mention about graal is the compilation time, which is still enormous. Since it is usually only done somewhat infrequently against the final product, it's tolerable for the gains in many use cases, but not sure how you feel about that and whether it's a good enough win to remove the runtime dep and get speedy startup.

@a4z
Copy link
Contributor

a4z commented Feb 1, 2021

Thanks for the info Joseph!

Having binary dependencies to what can be expected on a machine is fine, I guess.
But needs to be tested for various Linux distributions since they are more diverse. And for Windows it might be required to include some runtime ... but we will see.

The license seems also to be fine, not an expert either, but after a first glance there should not be any problem. Apache is compatible with GPL.

Anyway, if you could add usage of GraalVM with an option to sbt, something like sbt compile --with-graal, that would be very awesome.
And I guess all the other commands have to work similar, sbt it:test --with-graal , sbt assembly --with-graal
(if a --with-graal is sbt style, else just adopt to sbt style)

We can focus on the Mac version to start with, since I guess that is the most unified platform, and add the other OSes as we go and iterate on them over time.

@freitass
Copy link
Contributor

freitass commented Mar 18, 2021

I'd love to be able to install djinni from Homebrew, specially since I'm already a Homebrew user and there are compatibility issues between asdf and brew. I just wanted to point out that none of the JDK options currently available in "vanilla" brew support arm64 architecture. I had to install the Azul JDK in order to load native shared libs on a Macbook Pro M1. It might just be a problem for the support lib, but maybe it should be taken into account when making a decision.

@a4z
Copy link
Contributor

a4z commented Jul 17, 2021

I did some trial formula local, to play around and evaluate what needs to be done

For a brew package we would need to make a cask , since I do not think it is worth the effort to make a package that is buildable (with all the java/scala/sbt dependencies)

but even for a cask, there is a problem

 brew audit --new-formula djinni-generator
djinni-generator:
  * GitHub repository not notable enough (<30 forks, <30 watchers and <75 stars)
Error: 1 problem in 1 formula detected

so there is currently no way of getting the djinni generator into brew.

See https://docs.brew.sh/Acceptable-Formulae for more information

As I see it now, we have 2 options:

  1. wait until we might reach the required numbers, then create a cask
  2. creating a tap , host it ourself, and distribute djinni-generator via that
    see https://docs.brew.sh/Taps for more info

I think option 2 would be a doable way.
But we need to agree on that, and if we do that either rephrase this issue, or make a new issue, so the definition of done can be adopted.

As for the java dependency, I asked for that, and there is a gradle formula, that we can use as template, so this is not an impediment.

a4z added a commit to a4z/djinni-generator that referenced this issue Jul 18, 2021
@a4z a4z mentioned this issue Jul 18, 2021
a4z added a commit that referenced this issue Jul 19, 2021
Add documentation for  #26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
idea An idea that needs further discussion/refinement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants