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

Demonstrate usage of dev.nokee.jni-library plugin #154

Closed

Conversation

lacasseio
Copy link
Contributor

The Nokee plugins are a continuous effort to improve native support in Gradle. The JNI library plugins aim to offer the same quality as any Gradle core plugins but strictly aimed at native support.

This PR shows how the JNI library plugin can be used to build darklaf. Some custom logic was mixed inside the ad-hoc jni-library plugin which was based on a sample Paul and I worked on. I provided a reference implementation (sorry for providing it in Groovy, my Kotlin skills aren't that great) of the logic extracted out that works with the Nokee plugins. Along the way, there we missing API so I added them into a nightly build (version 0.3.0-<sha>). The documentation for those new APIs should appear over the next couple days in the nightly documentation. Let's discuss anything that feels wrong with this code and add any potential missing API to the JNI library plugin.

There is some current limitation but I don't see them as an issue for this migration:
1- Lack of source layout configuration.
2- Lack of build type modelling. I didn't see any obvious usage of the debug variant of the native library. If I'm mistaken, let's chat about adding this modelling.

Ideally, we would wait for the GA version, that would be version 0.3, before merging but we come back with another PR to update the version.

@lacasseio lacasseio force-pushed the lacasseio/use-nokeedev-plugins branch from f46a877 to a4bd676 Compare April 28, 2020 22:26
@weisJ
Copy link
Owner

weisJ commented Apr 29, 2020

Thank you very much. I guess I'll rewrite the build script in Kotlin eventually, but for now Groovy will do fine :).

Some questions:

  • Is there the option to get the name of the generated library? Currently the name of the fallback file is hardcoded in the gradle.properties file. Ideally this wouldn't be necessary.
  • What is the current state of downloading unbuildable variants from an exnternal repository e.g. github packages or maven?

Some things not directly related to the PR specifically. I am currently encountering the problem that the dynamic library produced for macOS doesn't include the currect load commands, (see #142) which may be due to the fact that currently it is build using the shared flag instead of dynamiclib.
What are the default parameters when building the dynamic library for objective-c?

@lacasseio
Copy link
Contributor Author

For the name of the generated library, there isn't an official way but I can definitely add the API. There are several part to a native library name. Considering libfoo.so (or libfoo.dylib or foo.dll) there is:

  • The prefix (e.g. lib on *nix system)
  • The base name (e.g. foo)
  • The extension (e.g. so, dylib, dll)
  • The soname additional part (e.g. API version and library version)
    For your use case, what is the part you need? The full filename or do you need to decompose the file name into the various parts?

Downloading unbuildable variants is still an open question. At the present moment, how the use case is solved within darklaf, it seems to be more a matter of workflow than addressing the root issue. I would probably stay with the current implementation see where we can evolve the requirement based on other use cases.

As for the shared vs dynamiclib, Gradle always uses -shared flag for a shared library. I couldn't find a satisfying answer on the difference between both flags. Could you explain why you would need dynamiclib over shared?

@weisJ
Copy link
Owner

weisJ commented May 1, 2020

I only need the name of the resulting binary (e.g. for a shared library) as this would avoid the manual configuration of the fallback lookup path.

Could you explain why you would need dynamiclib over shared?

I can't say for sure that it would make a difference, but the problem I am facing is that the dynamic library produced for macOS currently doesn't have the LC_VERSION_MIN_MACOSX load command set.

cmd LC_VERSION_MIN_MACOSX
cmdsize  16
version   10.14
sdk         10.14

This flags needs to be set and have the correct sdk version for some features to be enabled. I haven't checked with the library produced from this PR as I don't have access to a machine with macOS.

@lacasseio
Copy link
Contributor Author

Great point regarding the resulting binary file name. It is possible to access the linkedFile from the buildable variant link task. As an optimization, most unbuildable variants aren't created and aren't accessible via the views. This is an open question as we have been to the extreme of creating all variants (before the software model) and it had significant performance impact. On the other side configuring less is always better in terms of performance. Given the Nokee plugins tries it's best to use all lazy API offered by Gradle, unbuildable variant wouldn't affect the performance as they would only be configured if the variant is used. Rambling aside, I assume you would need the filename for both buildable and unbuildable variants?

As for the LC_VERSION_MIN_MACOSX load command, I would assume it's configured by -mmacosx-version-min. Gradle doesn't configure this flag so it would use the default value of the compiler. You can change it via the compilerArgs of the compile tasks. Unfortunately, there is no specialized modelling for macOS minimum version. Could you please open an issue on https://github.com/nokeedev/gradle-native with your use case. I can try to find a parallel with iOS development.

@weisJ
Copy link
Owner

weisJ commented May 7, 2020

I assume you would need the filename for both buildable and unbuildable variants?

Yes exactly. Would it be feasible to enable configuration creation regardless of whether they are buildable?

As for the LC_VERSION_MIN_MACOSX load command, I would assume it's configured by -mmacosx-version-min. Gradle doesn't configure this flag so it would use the default value of the compiler. You can change it via the compilerArgs of the compile tasks. Unfortunately, there is no specialized modelling for macOS minimum version. Could you please open an issue on https://github.com/nokeedev/gradle-native with your use case. I can try to find a parallel with iOS development.

I knew about the -mmacosx-version-min but had issues with the flags not actually being written to binary. I am currently trying to adapt this PR to auto-dark-mode to do check what options need to be set, but I am getting a build error where the JavaVM framework isn't found. Do you know what might cause this?

Cause 1: org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find dev.nokee.framework:JavaVM:10.15.
Searched in the following locations:
    - https://repo.maven.apache.org/maven2/dev/nokee/framework/JavaVM/10.15/JavaVM-10.15.pom
    - http://localhost:49418/dev/nokee/framework/JavaVM/10.15/JavaVM-10.15.module
Required by:
    project :auto-dark-mode-macos
Cause 2: org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find dev.nokee.framework:JavaVM:10.15.
Searched in the following locations:
    - https://repo.maven.apache.org/maven2/dev/nokee/framework/JavaVM/10.15/JavaVM-10.15.pom
    - http://localhost:49418/dev/nokee/framework/JavaVM/10.15/JavaVM-10.15.module
Required by:
    project :auto-dark-mode-macos

@lacasseio
Copy link
Contributor Author

Could you tell me what's the version you see when you run xcrun --show-sdk-version. If you could execute the build with the --info log and provide a build scan, I can probably figure out the issue. Just open an issue on https://github.com/nokeedev/gradle-native and I will have a look. It may be that the SDK version is different or something else. I want to improve the framework support to allow dynamic version by providing the maven-metadata.xml so you can use range version and the like.

For unbuildable variants, it's tricky went it comes to configuration. Users may make the assumption that some model elements are always available when they work on a system where the variant is buildable then hit some strange provider errors on system where the variant is buildable. That being said, I did some experimentation when creating the Xcode IDE plugin and I believe it would be possible to lazily create all variants and realize only the one that are requested to build. It would keep a good performance while allowing to "get" all variants for decision making. I will slowly work toward this solution while progressing on iOS and ensure the feature is cross-cutting to support the same in JNI. I would suggest keeping your workaround at the moment.

@weisJ
Copy link
Owner

weisJ commented May 7, 2020

xcrun --show-sdk-version
> 10.15.4

Build log:
https://gist.github.com/weisJ/e0636bb65bff546165be4b64c4246243


That being said, I did some experimentation when creating the Xcode IDE plugin and I believe it would be possible to lazily create all variants and realize only the one that are requested to build. It would keep a good performance while allowing to "get" all variants for decision making. I will slowly work toward this solution while progressing on iOS and ensure the feature is cross-cutting to support the same in JNI. I would suggest keeping your workaround at the moment.

Good to know 👍 Thank you for the effort.

@lacasseio
Copy link
Contributor Author

:-( That is what I thought The requested framework 'JavaVM' version '10.15' doesn't match current SDK version '10.15.4'. You can fix it temporarily by using 10.15.4 instead of 10.15. The embedded servers try as much as possible to complete the missing metadata like inferring the version. Down the line, I'm thinking of preprocessing all the headers to speed up compilation as well as inferring transitive dependencies. Unfortunately, I'm winding down the release 0.3 for next week. I will schedule it for 0.4: nokeedev/gradle-native#34

@lacasseio
Copy link
Contributor Author

As a side note, I tried the build on my machine (where my SDK version is 10.15) and it built without issue.

@weisJ
Copy link
Owner

weisJ commented May 7, 2020

Works fine now. I think I remember reading that github upped the default sdk version for their hosted runner to 10.15.4 a few days ago. It also contains the correct load command

       cmd LC_BUILD_VERSION
   cmdsize 32
  platform 1
       sdk 10.15.4
     minos 10.15
    ntools 1
      tool 3
   version 556.6

Previously the SDK had an incorrect value:

       cmd LC_BUILD_VERSION
   cmdsize 32
  platform 1
       sdk 11.3.1
     minos 10.14
    ntools 1
      tool 3
   version 530.0

Appareantly LC_BUILD_VERSION supersedes the old LC_VERSION_MIN_MACOSX. I'll still need to check if it actually does what its supposed to do but it looks promising.

@weisJ
Copy link
Owner

weisJ commented May 7, 2020

Other question:

When configuring the linker task

linkTask.configure {
    linkerArgs.addAll("-lobjc", "-framework", "AppKit")
}

I guess the -lobjc isn't actually necessary anymore as I used it to hijack to the cpp plugin for Objective-Cpp. Also is there a way to model the AppKit framework using a nativeImplementation?

@lacasseio
Copy link
Contributor Author

Unfortunately, Objective-C compilation doesn't imply any default libraries. According to my experimentation, -lobjc is still required: https://nokee.dev/docs/nightly/samples/java-objective-c-jni-library/

For AppKit, you should be able to use the same concept that JavaVM:

library {
    dependencies {
        nativeImplementation 'dev.nokee.framework:AppKit:10.15.4'
    }
}

Again, the version needs to match the SDK version of the machine, it's a cross-cutting issue that will be solved for all frameworks.

@lacasseio
Copy link
Contributor Author

Also writing the documentation for building JNI projects, I will go ahead and change to create all variants as it's very hard to explain the current behaviour and pose more issue than what is worth: nokeedev/gradle-native#36

@weisJ
Copy link
Owner

weisJ commented Jun 1, 2020

Closing due to #173.

@weisJ weisJ closed this Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants