Killing Ant. Replacing with java support for executing compilation linking and packaging. Also MSVC toolchain target

[Tom-Ski]

Ant kill
Ant is pretty annoying to maintain, and restrictive for things that we want to do, like separating out build targets for ios sim/device targets. No more ant scripts! All of it can be done logically in jnigen api to be executed via code/gradle.

MSVC support
Motivation for this:
Some third party libraries that don't expose source, or are too highly chained to MSVC make it impossible to use with jnigen, due to abi differences and name mangling not being compatible with mingw etc cross compilers on windows. As a fix for this, we can have a separate MSVC build target, so we can still have jni bindings that can target the same code base but just with a different build backend for when libs/source of external libraries are only MSVC compatible.

Features/Changes

• Ant killed, replaced with build processes directly in jnigen
• MSVC support added for windows targets
• CompilerABI type - gcc/msvc
• TargetType - device/simulator (ios only really)
• Refactored Gradle plugin for some cleaner less confusing methods of configuring
• Extract out global properties into build config, such as xcframework properties/version etc
• Refactored so BuildTarget is specific to an arch/bitness/target type unit.
• Ability to target specific ios slices with different configurations
• Remove abi from global configs on BuildTarget as now is a property of adding an android target.
• Add defaults to android/ios to build all default targets if we don't need to customize per slice/abi
• Gradle task of jnigenGHA, which generates a handy github workflow to build cross platform
• Gradle tasks are split into jnigen, (build jni structure and cpp files), build (compile and link), and package (bundle into jars)
• Multithreading compilation support

Todos after merging c gen PR

• Check 16k android
• Artifact publishing
• Optional packaging, hard errors in CI (Gha can add this)
• Hook commented tasks back up

[Tom-Ski]

@PokeMMO @SimonIT @Berstanio this is ready for a review now, I've added a list of all the things that have changed in this in the PR description

[badlogic]

How bout our lord and savior CMake? :p

[badlogic]

*ducks and covers*

…

On Wed, Jul 10, 2024, 11:22 Berstanio ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/BaseToolchain.java <#69 (comment)>: > + checkForTools(); + + collectCFiles(); + collectCPPFiles(); + cleanBuildDirectory(); + createBuildDirectory(); + + compile(); + } + + public abstract void compile (); + + private void collectCFiles () { + File rootDir = config.jniDir.file(); + if (!rootDir.exists()) { + throw new RuntimeException("Jni directory does not exist at path " + config.jniDir.file().getAbsolutePath()); The jniDir doesn't have to exist, if we e.g. have an exclude like this "../someDir/". Furthermore, since we now moved the jniDir to the "build" dir, maybe its not a good idea to start there, but maybe the project root instead? ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/BaseToolchain.java <#69 (comment)>: > + } + } catch (IOException e) { + logger.error("Failure to copy files into build directory", e); + throw new RuntimeException("Failure to copy files into build directory"); + } + + } + + public static List<File> collectFiles (Path rootDir, String[] includes, String[] excludes) throws IOException { + List<File> matchedFiles = new ArrayList<>(); + + Files.walkFileTree(rootDir, new FileVisitor<Path>() { + @OverRide + public FileVisitResult preVisitDirectory (Path dir, BasicFileAttributes attrs) { + for (String exclude : excludes) { + if (dir.endsWith(exclude)) { Excludes can also reference single files. ------------------------------ In gdx-jnigen-gradle/src/main/java/com/badlogic/gdx/jnigen/gradle/JnigenExtension.java <#69 (comment)>: > + + checkForTasksToAdd(target); + + } + + private Set<Os> osLevelTargetsSeen = new HashSet<>(); + private Set<Platform> platformLevelTargetsSeen = new HashSet<>(); + + private void checkForTasksToAdd (BuildTarget target) { + + Os os = target.os; + Platform platform = os.getPlatform(); + Architecture architecture = target.architecture; + Architecture.Bitness bitness = target.bitness; + + if (!osLevelTargetsSeen.contains(os)) { The jnigenBuild* tasks don't depend on jnigen anymore ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/BaseToolchain.java <#69 (comment)>: > + @OverRide + public FileVisitResult preVisitDirectory (Path dir, BasicFileAttributes attrs) { + for (String exclude : excludes) { + if (dir.endsWith(exclude)) { + return FileVisitResult.SKIP_SUBTREE; + } + } + return FileVisitResult.CONTINUE; + } + + @OverRide + public FileVisitResult visitFile (Path file, BasicFileAttributes attrs) { + String filePath = file.toString(); + boolean include = false; + for (String includePattern : includes) { + if (filePath.matches(includePattern.replace("**", ".+"))) { path/*.cpp is also a valid path, if you don't want to pick up recursivly in sub directories ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/IOSToolchain.java <#69 (comment)>: > + } + + args.add("-arch"); + args.add(getClangArch()); + + String iosVesion = target.targetType == TargetType.DEVICE ? "-miphoneos-version-min" : "-mios-simulator-version-min"; + args.add(iosVesion + "=" + config.robovmBuildConfig.minIOSVersion); + + args.addAll(stringFlagsToArgs(target.cFlags)); + + + args.add("-Ijni-headers"); + args.add("-Ijni-headers/" + target.os.getJniPlatform()); + args.add("-I."); + args.add("-g"); + args.addAll(Arrays.asList(target.headerDirs)); Should be prefixed with "-I" ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/GNUToolchain.java <#69 (comment)>: > + strip(); + } + + + private void compileC () { + if (sourceCollectedCFiles.isEmpty()) { + logger.info("No c files, skipping c compilation"); + return; + } + + List<String> args = new ArrayList<>(); + args.addAll(stringFlagsToArgs(target.cFlags)); + args.add("-Ijni-headers"); + args.add("-Ijni-headers/" + target.os.getJniPlatform()); + args.add("-I."); + args.addAll(Arrays.asList(target.headerDirs)); Should be prefixed with "-I" ------------------------------ In gdx-jnigen/src/main/java/com/badlogic/gdx/jnigen/build/toolchains/BaseToolchain.java <#69 (comment)>: > + checkForTools(); + + collectCFiles(); + collectCPPFiles(); + cleanBuildDirectory(); + createBuildDirectory(); + + compile(); + } + + public abstract void compile (); + + private void collectCFiles () { + File rootDir = config.jniDir.file(); + if (!rootDir.exists()) { + throw new RuntimeException("Jni directory does not exist at path " + config.jniDir.file().getAbsolutePath()); Yeah, thinking more about it, the cIncludes are really difficult to work with now, since you still can't do releative path like ../, but now you are in the build directory, where you can't commit files into. — Reply to this email directly, view it on GitHub <#69 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAD5QBHHX2B4C5JZ3N4VMTTZLT4LRAVCNFSM6AAAAABJYL6ML6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRYGQ3DCMBVGU> . You are receiving this because your review was requested.Message ID: ***@***.***>

[Berstanio]

Another thing I notices, where I don't really figure out easily why, jnigenPackageAndroid_xxx still packages all android abis. But not really an issue

[Berstanio]

Do we maybe want to retain these fields, mark them as deprecated and just do:
static public Os os = HostDetection.os;

[Tom-Ski]

Yeah, would make adoption a bit easier