cordova: added Cordova binding based on Java binding

[sunlin-link]

The Cordova plugins for each Java packages will be created if BUILDCORDOVABINDING=ON and BUILDSWIGJAVA=ON.

[sunlin-link]

@dnoliver Here's the repo of upm-cordova-binding. Could you tell more abou the Travis CI job

[dnoliver]

@sunlin-link thank you for the repository link!

I was thinking about adding a Travis Job for building upm-cordova-bindings. I can help with that, you do not need to do it. It will execute the cordova binding generation for each pull request that we submit, so we will know whenever the process is broken. If you want to explore it, you can look at the .travis.yaml file.

[sunlin-link]

@dnoliver Is this pull request okay to merge?

[sunlin-link]

@dnoliver @pylbert Any other comments?

[sunlin-link]

@pylbert Is this pull request ok to merge?

[pylbert]

I'm having trouble understanding why the CMake generation step does a check for upm-cordova-bindings and tells the user to install ' upm-cordova-binding':

CMake Error at cmake/modules/FindCordova.cmake:7 (message): Unable to find the npm package for building cordova bindings, please install by 'npm install -g upm-cordova-binding'

and later, a make target ALSO tries to run npm install -g upm-cordova-binding:

[ 97%] Building cordova bindings based on swig java modules npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules/upm-cordova-binding npm WARN checkPermissions Missing write access to /usr/local/lib/node_modules /usr/local/lib └── upm-cordova-binding@0.2.0

npm ERR! Linux 4.4.0-97-generic npm ERR! argv "/usr/local/bin/node" "/usr/local/bin/npm" "install" "-g" "upm-cordova-binding"

Do we need both of these, the second one seems redundant? Why does the build target try to install?

On Mon, Feb 26, 2018 at 7:37 PM, Lin Sun notifications@github.com wrote:

@pylbert https://github.com/pylbert Is this pull request ok to merge?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/intel-iot-devkit/upm/pull/618#issuecomment-368736673, or mute the thread https://github.com/notifications/unsubscribe-auth/AOqVeT0mVg3nneUPKuHQHHJvMvX8l5Ixks5tY3hzgaJpZM4QoH1- .

[sunlin-link]

Thanks for you comments! Indeed the second one is redundant after the installation is checked during the cmake procedure. I've removed the line for installing the upm-cordova-binding module.

[pylbert]

I no longer get prompted to install the cordova binding however I see a failure in generator.js:

[ 31%] Building cordova bindings based on swig java modules
/usr/local/lib/node_modules/upm-cordova-binding/src/generator.js:40
                        throw Error("Unsupported format");
                        ^

Error: Unsupported format
    at Error (native)
    at /usr/local/lib/node_modules/upm-cordova-binding/src/generator.js:40:31
    at Array.forEach (native)
    at /usr/local/lib/node_modules/upm-cordova-binding/src/generator.js:29:62
    at Array.forEach (native)
    at /usr/local/lib/node_modules/upm-cordova-binding/src/generator.js:24:20
    at Array.forEach (native)
    at Object.generatePlugin (/usr/local/lib/node_modules/upm-cordova-binding/src/generator.js:6:31)
    at Object.<anonymous> (/usr/local/lib/node_modules/upm-cordova-binding/bin/upm-cordova-binding:39:11)
    at Module._compile (module.js:570:32)
src/CMakeFiles/cordova_binding.dir/build.make:57: recipe for target 'src/CMakeFiles/cordova_binding' failed
make[2]: *** [src/CMakeFiles/cordova_binding] Error 1
CMakeFiles/Makefile2:979: recipe for target 'src/CMakeFiles/cordova_binding.dir/all' failed
make[1]: *** [src/CMakeFiles/cordova_binding.dir/all] Error 2
Makefile:138: recipe for target 'all' failed
make: *** [all] Error 2

[pylbert]

@sunlin-link, is the UPM cordova binding work still relevant? If not, can we close these #618 and #633?

[sunlin-link]

@pylbert I rebased my upm commit onto latest master branch and rebuilt upm with enabling cordova binding. But I couldn't reproduce your error above. However, I reviewed your pull request for upm-cordova-binding and think your change is right because I did ignore the interface keyword.

First of all, I want to confirm whether this pull request itself is okay. Becuase it's just about integrating the cordova binding tool to upm's building process, but not about the tool itself. Even the tool works well at the moment, it may still have new problem over the update of upm and it's my responbility to keep update in the future.

So I wonder if we could just get this pull request merged, but as a follow-up we're having a blocking issue that I need to review the impact of the interface keyword, and as I mentioned in your pull request for upm-cordova-binding, I need to wait until the movement of the repo is finished.

[pylbert]

Here is a branch which incorporates @dnoliver's additional requests as well as a travis-ci target for building the cordova UPM bindings (which fails): https://github.com/pylbert/upm/commits/cordova

[sunlin-link]

@pylbert As for pull request #633, I think it's same with this pull request. If the change of this pull request is okay, #633 is okay as well. If the cordova binding generation has errors, we should raise another issue on the upm-cordova-binding repo. I also think there should be something to prevent cordova binding genration from broking the whole process. Becuase the update of upm is allways first, I can update the cordova binding tool only after I see any error happened.

[pylbert]

@sunlin-link, #633 added onto #618 (@dnoliver's additional requests, as well as creating a travis-ci target for cordova which shows the failures WRT the interfaces). Please use #633 for testing the cordova binding generation. I pushed PR1 to your personal repo which gets past the interface errors in travis-ci but I imagine there may be additional logic needed to make them work. If you can merge/test PR1 we can get this moving. Alternatively I can pull upm-cordova-binding into intel-iot-devkit and work from there if you're OK with that.

I like your idea of adding something to prevent your cordova binding generation code from breaking the whole process (possibly add a versioning check into CMake for the binding generator). Maybe you can incorporate all the above into this PR? Remember to sign your commits and provide descriptive text in your PR. Here is additional info on contributions.

[sunlin-link]

Which version of node.js are you using? I checked out https://github.com/pylbert/upm/commits/cordova and run cmake with the option -DBUILDSWIGJAVA=ON -DBUILDCORDOVA=ON and then run make, I didn't see any error. I'm using node.js 6.11.3 because I had building error of upm when using version 8 and 9.

[Propanu]

Our CI infrastructure runs Node 4,5,6 builds. For newer versions of Node, there's a SWIG patch that should allow you to build the libraries, see this thread.

[pylbert]

The NodeJS version is not the problem here. UPM builds Java interfaces for SWIG >= 3.0.9 (e.g. the UPM docker images) and classes for SWIG < 3.0.9. @sunlin-link's generator does not handle the 'interface' keyword. In #633 I added a line item for the cordova binding build (and other requested changes) so others could see the failures.

If you would like to see them:

1. pull #633 and push a commit for travis-ci to run or
2. run the docker build or
3. update to SWIG >= 3.0.9. or
4. see failure below

/root/.nvm/versions/node/v5.12.0/lib/node_modules/upm-cordova-binding/src/generator.js:40
                        throw Error("Unsupported format");
                        ^
Error: Unsupported format

[sunlin-link]

Thanks for your procedure! I've reproduced the error by updating swig.

The repository of upm-cordova-binding has been moved to new location managed by intel. So we could discuss it over there.

I've already submit a minor fix to eliminate the error. So if you update the module and run it again, the failure should disappear. I'm still checking potential impact of the interface keyword, but for the moment it will not break the building process anymore.

[pylbert]

@sunlin-link, closing - merged in f45429e1f0f089c40f1159890f30a89a94eb64ce, b09944f4b85eb1278c2aaeb1638099b04ebfc106, and 757683b2caa52d4f873a26efa28d6eaaeb019991.