Skip to content

Work towards #4 and replace the cmake-maven-plugin with the maven-ant… #15

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Apr 3, 2019
Merged

Work towards #4 and replace the cmake-maven-plugin with the maven-ant… #15

merged 12 commits into from
Apr 3, 2019

Conversation

bmarwell
Copy link

@bmarwell bmarwell commented Apr 2, 2019 •
edited by tresf
Loading

Working towards #4 and replacing the cmake-maven-plugin with the maven-antrun-plugin.

  • Options from previous cmake-plugin-execution added.
  • Working directories added.
  • Environment variables still need to be done.
  • Added another test which currently fails (to highlight a known problem).
  • Not tested on MacOS or Windows. (Edit: @tresf done Apr 3)

@bmarwell
Work towards #4 and replace the cmake-maven-plugin with the maven-ant…
c1e1544 
…run-plugin.

  - Options from previous cmake-plugin-execution added.
  - Working directories added.
  - Environment variables still need to be done.
  - Added another test which currently fails (to highlight a known problem).
  - Not tested on MacOS or Windows.
@bmarwell bmarwell requested a review from tresf April 2, 2019 07:51
@bmarwell
Copy link
Author

bmarwell commented Apr 2, 2019

Please ignore the test failure. It would also fail on the current maven branch. I only added this test to keep track of the static native method which isn’t working right now.

Other than that, I also added some cleanup by moving the version numbers into properties, which might some day go into a parent project’s pom.xml.

Please test the configuration on windows if possible, I hadn’t had the chance to do so.

bmarwell added 4 commits April 2, 2019 10:36
@bmarwell
Pass -D args after double dash to cmake, so they become buildtool arg…
548cd23 
…uments, not cmake arguments.
@bmarwell
use -D args only for makefile generation, not for the build/compile p…
ef5b72e 
…hase.
@bmarwell
Added variables for hard-coded paths where needed.
6c49614 
  - Moved directory src/java -> src/main/java according to maven conventions.
  - Moved directory src/cpp  -> src/main/cpp  according to maven conventions.
  - Created variables for JNI output and include paths.
  - Created variables for CPP source directory.
  - reduced the --build and buildir arglines to a single argline.
@bmarwell
Ignore test for now to see successful compilation in travis-ci.
c77563e
@bmarwell
Copy link
Author

bmarwell commented Apr 2, 2019

Ok, the commit ef5b72e fixed passing wrong options to MacOS (and probably Windows) builds. The -D and -G generator options are now only passed for the generate phase, not to the compile phase (--build).

In commit 6c49614 I added more variables, so the CMakeLists.txt file does not need to rely on hardcoded paths.

Also, I deactivated the test I mentioned earlier in commit c77563e. It‘s still there, but @Ignored for now.

bmarwell added 3 commits April 2, 2019 16:47
@bmarwell
revert to previous directory structure for now, so we can accept exis…
f152549 
…ting pull requests.
@bmarwell
Add animal sniffer plugin to make sure this code only uses the Java 6…
e12ba6c 
… API. Makes no assumptions for dependencies.
@bmarwell
Add the version from pom.xml to the list of variables and use it for …
90fb0a3 
…the native lib.

  - Defined JSSC_VERSION variable.
  - Enabled warnings (compiler options)
  - Fixed jclass / jobject parameter in the .cpp files.
  - Will re-enable the test.
@bmarwell
Copy link
Author

bmarwell commented Apr 3, 2019

Added two commits.
e12ba6c will add the animal sniffer plugin to make sure we only use Java 6 API for now.

90fb0a3 will define a version variable as requested in #17 (but will not use it for cmake’s PROJECT_VERSION). It also fixes a method call with a wrong parameter (before: jobject instead of jclazz for static java method) an thus re-enables the @Ignored test.

I also added compiler warnings (/W4 /WX on Windows, -Wall -Wextra -pedantic -Werror on others).

Is there a way to make sure that jssc.cpp overwrites all methods defined in the generated jssc.h?

@tresf tresf merged commit f4170b0 into maven Apr 3, 2019
@tresf tresf deleted the maven-cmake-antrun branch April 3, 2019 23:33
@tresf tresf mentioned this pull request Apr 5, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tresf tresf tresf approved these changes

Assignees

@tresf tresf

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bmarwell @tresf