weisJ / auto-dark-mode

IDEA plugin to automatically apply system theme settings on macOS and Windows.
https://plugins.jetbrains.com/plugin/14076-auto-dark-mode
MIT License
53 stars 14 forks source link

Resolve issues regarding Nokee nightly release 0.4 #5

Closed lacasseio closed 4 years ago

lacasseio commented 4 years ago

This PR should solve the issue you are seeing with the nightly release for 0.4: https://github.com/nokeedev/gradle-native/issues/45

The uber JNI JAR plugin was arranged to support the change with 0.4 around creating all variants regardless if they are buildable or not as well as differing the variant creation as late as possible. A new API was added to query if a binary is buildable or not.

Feel free to request any API that may help clean up the implementation code. Through this PR, I opened these new issues for APIs that I feel are missing: https://github.com/nokeedev/gradle-native/issues/48 and https://github.com/nokeedev/gradle-native/issues/47. Your comments are welcome.

weisJ commented 4 years ago

Thank you vary much for your effort. I am experiencing three (new) issues here:

lacasseio commented 4 years ago

Let me have a look at those issues.

lacasseio commented 4 years ago

This took a bit more time than expected. I was displeased with the code I wrote for the Uber JNI JAR plugin. The main issue resolve around the conflict between the defaults provided by the plugins and the darklaf defaults (uber JAR + use prebuilt binaries for unbuildable variants). For this to work without much compromised, I introduced a new API, JniLibrary#getNativeRuntimeFiles() that can be overwritten. It simplifies the use case for using prebuilt binaries. For the uber JNI JAR use case, I simply adapted the code a little bit. Both configurations are separated plugins. It should solve the first and second issues.

For the third issue, I think something is going on with how the Xcode installation is selected. I downloaded Xcode 10.3 and will debug this further tomorrow.

Thanks for trying out the nightly, I really appreciate it.

lacasseio commented 4 years ago

I forgot to mention if you are able to know the path of the prebuilt binaries on GitHub action, you could write a custom task that downloads them directly from the Web. Since the getNativeRuntimeFiles() is a FileCollection, you could do something like:

def prebuiltBinariesTask = task.register('downloadPrebuiltBinary', DownloadPrebuiltBinaryFromGitHubAction) { ... }
library.variants.configureEach {
    if (!sharedLibrary.buildable) {
        nativeRuntimeFiles.setFrom(prebuildBinariesTask.flatMap { it.prebuiltBinaryFile })
    }
}

It would require each binary to have its own tasks and wired to the right variant. For this, I will release a configureEach(Spec, Action) API soon which will allow you do select which variant to configure like:

library.variants.configureEach({ it.targetMachine.architecture.'64Bit' }) {
    // configure only 64-bit architecture variants
}
lacasseio commented 4 years ago

@weisJ I pushed the last of the changes that should get you on the right track. The CI is all green and it seems to me the code is much cleaner. If you seen anything that you would wish to be nicer, down hesitate to open an issue on the project.

weisJ commented 4 years ago

I forgot to mention if you are able to know the path of the prebuilt binaries on GitHub action, you could write a custom task that downloads them directly from the Web.

Sounds great. I’ll have a look at how I can incorporate it.

Everything is working now so that’s great. I think there should be an option to model the used SDK version somehow. Right now it correctly uses the correct framework versions but still uses the 10.15 SDK for compilation.

Working directory: /usr/bin Command: /usr/bin/clang++ -m64 -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -x objective-c++ -dM -E -v -

The result is that somehow -mmacosx-version-min=10.14 has no effect in on the load commands of the binary. I don’t actually know why this is happening in the first place because from the documentation -mmacosx-version-min should result in a changed minos flag. Maybe there is some incompatibility between the 10.15 SDK and earlier macOS versions that prevent the flag from having an effect. However I’ll open a separate issue for this.

lacasseio commented 4 years ago

You are right, I will circle back to the SDK modelling when I get back in improving iOS support. The direction I will take this is to model the "owning" SDK of the dependencies via attributes. The framework discovered via Nokee will have the SDK information attached to them so you can depend on the proper SDK.

With regards to version conflict, I believe Gradle has already everything to force a version and/or warn when the incorrect version is used. I think ResolutionStrategy#eachDependency could help with this scenario.

weisJ commented 4 years ago

With regards to version conflict, I believe Gradle has already everything to force a version and/or warn when the incorrect version is used. I think ResolutionStrategy#eachDependency could help with this scenario.

Do you mind explaining how this could be done? Is the SDK modeled as a dependency in nokee?

~~Also I have seen in the compiler log that the sdk is passed using -isystem and a direct path to the include folder. However the preferred way to do this would be to pass it using -isysroot just with the path to the sdk. So instead of -isystem /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include we would have -isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk I am not sure though if there is a difference between the two. Maybe both need to be specified.~~

Edit: Seems like this is actually the case and I have simply checked the wrong arguments.