Tom-Ski
commented
2 weeks ago @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
Open Tom-Ski opened 1 month ago
Tom-Ski
commented
2 weeks ago @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
commented
2 weeks ago How bout our lord and savior CMake? :p
badlogic
commented
2 weeks ago 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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671884937:
- 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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671888168:
- }
- } 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
collectFiles (Path rootDir, String[] includes, String[] excludes) throws IOException { - List
matchedFiles = new ArrayList<>(); - Files.walkFileTree(rootDir, new FileVisitor
() { - @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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671897875:
+
- checkForTasksToAdd(target);
- }
- private Set
osLevelTargetsSeen = new HashSet<>(); - private Set
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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671901922:
- @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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671916020:
- }
- 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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671916145:
- strip();
- }
- private void compileC () {
- if (sourceCollectedCFiles.isEmpty()) {
- logger.info("No c files, skipping c compilation");
- return;
- }
- List
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 https://github.com/libgdx/gdx-jnigen/pull/69#discussion_r1671909705:
- 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 https://github.com/libgdx/gdx-jnigen/pull/69#pullrequestreview-2168461055, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5QBHHX2B4C5JZ3N4VMTTZLT4LRAVCNFSM6AAAAABJYL6ML6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRYGQ3DCMBVGU . You are receiving this because your review was requested.Message ID: @.***>
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