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

Cannot build with Ninja CMake backend #73

Closed
caryoscelus opened this issue Nov 28, 2016 · 13 comments
Closed

Cannot build with Ninja CMake backend #73

caryoscelus opened this issue Nov 28, 2016 · 13 comments

Comments

@caryoscelus
Copy link

While building with make backend works fine, building with ninja fails with the following message:

ninja: error: 'library/so/tinyspline/tinysplinejava.java', needed by 'library/CMakeFiles/tinysplinejar.dir/java_compiled_tinysplinejar', missing and no known rule to make it

@msteinbeck
Copy link
Owner

Is the Java binding the one you are looking for? What does ninja say when you build a specific target, for instance, tinyspline_shared?

@msteinbeck
Copy link
Owner

It looks like CMake creates a Ninja file that does not handle dependencies between targets. The problem is that we have two targets, tinysplinejava (which creates all java files) and tinysplinejar (which creates a jar from the java files). Unfortunately, tinysplinejar is executed before tinysplinejava has been called. Therefore, tinysplinejar is not able to find library/so/tinyspline/tinysplinejava.java. A workaround is to invoke ninja tinysplinejava first and ninja afterwards.

@caryoscelus
Copy link
Author

Yeah, i already found that workaround after your first message :)

Noticed another minor problem: ninja clean target does not clean everything (e.g. library). This also means that after initial java build, even clean-building would not reveal the problem. And it may actually build old version of jar, i think.

@msteinbeck
Copy link
Owner

I'm not sure if I can fix this issue here. It looks like this is a CMake related issue.

@caryoscelus
Copy link
Author

I did some research and now think that it's likely CMake swig module issue. If it would provide a list of generated files, everything should've worked fine (then again i don't know if it's possible with the way CMake works). I have found nobody successfully combining cmake swig and add_jar, most developers seem to be using custom commands :/

That said, i have no idea why make currently works (while ninja fails), but i'm afraid that might be not intended behaviour because jar target relies on java files which cmake has no idea about.

@msteinbeck
Copy link
Owner

I have found nobody successfully combining cmake swig and add_jar, most developers seem to be using custom commands :/

I really would like to avoid that :).

That said, i have no idea why make currently works (while ninja fails), but i'm afraid that might be not intended behaviour because jar target relies on java files which cmake has no idea about.

So, how do we solve your problem?

@caryoscelus
Copy link
Author

I really would like to avoid that :).

Yeah, i totally agree with that.

So, how do we solve your problem?

I think the best way is to add support for output files in CMake Swig and then fix your CMakeLists accordingly. I'm currently looking if i can do that myself and then will be filing issue or PR upstream. If it doesn't work out for some reason, i have no idea what would be the best solution. Probably at least mentioning this problem somewhere in readme.

@caryoscelus
Copy link
Author

Hmm, minimal solution doesn't even require changing anything in your CMakeLists, though it doesn't fix necessity to list .java files manually. Will link the PR here when it's ready to track its status.

@msteinbeck
Copy link
Owner

[...] though it doesn't fix necessity to list .java files manually.

Actually, I had to add the list of generated swig files (source files, not the compiled so/ddl files) manually in order to let make clean remove them as well.

@caryoscelus
Copy link
Author

Hmm, with my fix some files are being cleaned, but not all.

But i meant the necessity to list files in jar target instead of writing target name or some variable there. Actually i'm a bit confused about it now, cause resulting jar contains more classes than just those.

@msteinbeck
Copy link
Owner

msteinbeck commented Nov 29, 2016

This is a feature of add_jar (or jar itself?). All depending Java files will be included to the archive as well making it easier to maintain the Java binding as newly introduced files must not be listed explicitly. On the other hand, tinysplinejava.java and tinysplinejavaJNI.java are always generated by Swig, regardless of the underlying binding.

@msteinbeck
Copy link
Owner

I have tested current CMake master and can confirm that your patch fixes the dependency issue between the java and jar target of TinySpline. Thanks for your work! However, (as you already mentioned) ninja clean does not remove all Java files from library/so/.... It looks like Ninja does not handle ADDITIONAL_MAKE_CLEAN_FILES properly. Nonetheless, due to the fixed dependency order the jar target should not build outdated files as they should be overwritten by the java target beforehand. I leave it to you to close this issue.

@caryoscelus
Copy link
Author

Ah, right, forgot to attend the issue here. You're welcome :)

Yeah, i don't think that those files can do any harm (except for hypothetical case where some class is deleted, but still referenced in source - i'm not swig expert to tell if that can actually happen), so it's safe to close this.

Generally i think that using ADDITIONAL_MAKE_CLEAN_FILES should be a last resort measure and it usually means that either your or upstream code is not doing something right. I would recommend to check the code and possibly file issues to cmake. (i'm likely to end up not using tinyspline, so won't be hacking it either)

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

No branches or pull requests

2 participants