Electron support

[p120ph37]

Re: Issue #2 & #4

[dkrutsko]

Woot, my hero!! I'll take a look, hopefully it resolves both issues :-D

[p120ph37]

I've tested with electron-rebuild on OSX. I have not validated that it works 100% on Windows and Linux yet, but it ought to be close enough to correct that you or I or whoever is working on those platforms next can fill in any missing bits easily.

[p120ph37]

Also note that once the node-gyp build method has been proven, you may want to remove your Makefile and use node-gyp to do your release builds so that there is only a single build system to keep up to date.

[dkrutsko]

Ideally, there should be an automated way to build all these binaries, like with Travis or AppVeyor, because at the moment, I'm building them all manually. It's just when I was building this library I went with what I knew and I wanted to get something out quickly. And I had bad experiences with node-gyp back when it first came out as well and I'm sure it's gotten better now.

Regardless, I want to take a close look at this entire library when robot 2.1 is released. Ideally I want to get everything over to ES6 and Nan as well as leverage more asynchronous functionality. But let's start with this and see if it solves the immediate issues with Electron/Webkit.

[p120ph37]

Had chance to test the windows build... there were some minor issues for which I found simple solutions, but there's a kinda major one that's giving me problems - apparently VS 2010+ throws fits if two source files (and thus also resulting obj files) have the same name, even if they are in different directories. See here for a general discussion of this issue:
http://stackoverflow.com/questions/3729515/visual-studio-2010-2008-cant-handle-source-files-with-identical-names-in-diff
In your project file, all of the source files have explicitly-assigned object output subdirectory names. I haven't yet seen an option to enable this behavior in node-gyp's generated project file. One hackish solution would be to rename the Addon source files to not share exact names with the Robot sources (e.g., "AClipboard.cc" rather than "Clipboard.cc" for the Addon one). Hopefully I can come up with something better than that though...

[p120ph37]

Okay, got it sorted out by using an intermediate .lib target instead of linking all the objects in one go. Someone needs to verify the build in Linux next.

[p120ph37]

Updated and tested Linux build support - now builds cleanly on all three platforms. :-D

[dkrutsko]

@p120ph37: Hey there, great work! I'm working on getting this merged but I have a few questions. The first is, do we really need the abi.js file? It's just returning the node ABI version, correct? Could it not simply be replaced by <!(node -e console.info(process.versions.modules)) in the binding.gyp file?

Second, while I know the package.json file was not modified, what's going to happen during a typical npm/yarn install robot-js? Will it try to compile the project because the binding.gyp file is present? or will it still use install.js and grab the binaries remotely? What happens if the install script fails because you're using an unsupported version?

Ideally, I want everything to work as it did before but have the node-gyp option available for people that are unable to install precompiled binaries, or are using electron, etc.

For robot-js 2.0, I'm thinking that we'll probably drop Node 0.12 support, convert everything to ES6, drop the custom project files in favor of node-gyp and maybe make use of nan (although the way I'm doing it now isn't the worst thing in the world, we'll see when Node 8 is released). But the pre-compiled feature is still great I think, it's how node-sass does it.

[p120ph37]

"do we really need the abi.js file?"

The problem is that node -e console.info(process.versions.modules) will return the ABI number of the system node binary, not of electron (or whatever other non-system node headers gyp may be compiling against). This is also why we can't just use the gyp variable node_module_version - because that will be the ABI of the system node binary, which is being used to run node-gyp, not the target binary for which we are compiling.

The other possible way (besides parsing the header files directly) would be to add a dev dependency on node-abi (which is just a look-up table), and do something like <!(node -e "console.info(require('node-abi').getAbi('<(target)','<(runtime)'))") But since the headers are already present, it seems not unreasonable to rely on them rather than hoping some third-party keeps their crib sheet up to date.

"what's going to happen during a typical npm install?"

The normal install.js script will run. There is no automatic node-gyp magic that kicks in. For people who want to build magic into their modules, there is the node-pre-gyp package, which basically does something like what you already do in your install.js (download precompiled binary if possible), but automatically falls back to a node-gyp build if necessary. It also contains some tooling to automate the maintainer's build-and-publish task. It might be something to look into in the future, but we'd have to add support to it for our non-https solution (currently it is mostly focused on publishing to an S3 host, and there is also an add-on module to support publishing to github).

"Ideally, I want everything to work as it did before but have the node-gyp option available for people that are unable to install precompiled binaries, or are using electron, etc."

Yep, that's what I was figuring too.

[dkrutsko]

Thank you for answering my questions and for all your hard work. I'll run a few more tests here then we should be good to go! And yes I agree, I'd like to look more into standardizing the development of this addon for the future with tools like node-pre-gyp and so on.

[karliky]

I love this, thanks @p120ph37 and @dkrutsko for the hard work!

[p120ph37]

@dkrutsko, I've done a bunch of work in p120ph37/robot-js/prebuilt to convert things to the node-pre-gyp way of doing things. There are some significant changes there, such as hosting the prebuilt binaries on github (something that I know you are reluctant to do in your mainline branch), and adding continuous-integration for automatic testing, building, and publishing. Currently, the "testing" amounts to just trying to run the "types" test to make sure the module loads correctly under each possible runtime/platform/arch, but at some point it might be interesting to actually use a second instance of Electron as the testing app so that all of the tests could be run in a completely non-interactive mode on the CI servers. During that work (especially thanks to the CI), I discovered a few small issues that would also affect this PR, and backported the fixes into my dev branch referenced in this PR. Another possible change I'm considering for my prebuilt branch is separating the Robot part of the gyp config out and submitting it as a PR to the upstream Robot project, and then using a git submodule link to reference a tag in the upstream Robot project directly, rather than having a separate copy of the Robot sources embedded in Robot-JS. Separating the gyp config for the Robot part would also help the Robot-JS build to insulate Robot from a bunch of the Node-version-specific gyp-default-options weirdness.

@karliky, if you would like to try out my prebuilt branch instead of my dev branch mentioned in this PR, feel free. I am using it in an electron project that I package for several different platforms from a single script - something not possible if you need to compile a module from source. (caveat: the multi-platform packaging tools for electron kinda suck right now - I had to do some horrible things to get electron-builder to correctly reinstall robot-js per target platform, but this will probably get sorted out as more people use the multi-platform packaging features. The authors of electron-builder prefer yarn to npm to work around some of this, but yarn is completely broken in other ways...)

[dkrutsko]

It's funny you mention yarn, if these rumors are true we might see an io.js-style merger when npm@5 comes out :-P

With regards to your changes, thanks again, I think you're doing incredible work with improving electron and compiler support. With regards to hosting binaries on GitHub, I'm already doing that but through a custom domain. For Continuous Integration, I'm a bit annoyed there isn't one CI that handles all three platforms for everything (unless something changed). Last time I looked, I needed both a Travis and AppVeyor system to handle compilation and testing on all three platforms. But for sure, the types, timer and maybe a few more tests can be performed automatically for integration testing. But I think you'd be hard-pressed to find something that can test the core of the library. Especially when some results require manual verification.

For separating robot from robot-js, I actually finally discovered that .gitmodules thing and I'm thinking it might be perfect for this. The last thing I want to do is make the compilation any more painful than it already is. But this stuff is pretty complicated so I'm not surprised that it might require a more advanced set up.

[p120ph37]

Yeah, the setup I have on my branch uses both Travis and AppVeyor, both of which are free for open-source projects. I'd have preferred to use just Travis for all platforms, but unfortunately they seem not to consider Windows a priority right now. AppVeyor isn't bad, but it's not open-source like Travis is, so you have to trust the docs a bit more rather that reading the source like you can for Travis. It also uses a config format that is very similar to Travis, but not exactly compatible, which can be confusing. I'm not sure what exactly is holding Travis back from releasing Windows support - they already use GCE, which is exactly what AppVeyor uses. They have commented that they are unhappy with the scripting available in Windows, but PowerShell seems to work well enough as shown by AppVeyor, and there's also the option of using Bash via Cygwin or msys/mingw...

I have successfully run the types and timer tests via automation, but for a quick sanity check, I have it just running the types test right now. I think all of the tests could technically be orchestrated with the help of a separate Electron process which would act as a target for automation, or with other a-priori knowledge like hard-coding the virtual frame-buffer size in the X start-up command, and then validating that the Screen object reads the same dimensions, but some of the tests would take a bit of work to set up and harvest results from a separate Electron process, so I haven't done anything too detailed yet.

[dkrutsko]

Yeah it sounds like a lot of work trying to get something automated up and running. But regardless, when I'm back from vacation I'd like to get Node 8 support out along with this PR integrated. Then we can focus on the new things.

Ideally, I still want to get the new C++ version of robot out with the new features before going full ES6 with robot-js but we'll see, some of the new features are hard to implement (like overlay windows and such).

[p120ph37]

Well, npm@5 is out now and rather broken for node-pre-gyp, yay?
mapbox/node-pre-gyp#298
More reason to hold off for a little longer on mainlining my prebuilt branch.

[karliky]

@p120ph37 @dkrutsko

Cant't get it to work on Macosx, seems that is trying to load a different native module, I did the ./node_modules/.bin/electron-rebuild thing and my node version is v6.2.1

Here is my package.json:

{
  "name": "machinima-studio-wow",
  "version": "1.0.0",
  "description": "",
  "main": "main.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1",
    "start": "electron ."
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "bulma": "^0.4.2",
    "robot-js": "git+https://github.com/p120ph37/robot-js.git",
    "electron": "^1.6.11"
  },
  "devDependencies": {
    "electron-rebuild": "^1.5.11"
  }
}

[dkrutsko]

@karliky You're going to need to compile robot-js for ABI version 53. The one you have, 48, is for Node 6. That being said, you should be able to use @p120ph37's version to compile using node-gyp. If you run into compilation errors, take a look at this PR. There's also in-depth compiling instructions available here.

I know it really sucks at the moment, and robot-js 1.1 will hopefully fix all these issues. It'll also add Node 8+ support, if it's currently broken. I just have to find the time to get everything merged in between the other projects I'm working on :-/

[p120ph37]

@karliky, if you use my "prebuilt" branch, you don't need to use "electron-rebuild" at all. That tool is for rebuilding from source (which is possible for each platform natively, but the point of my "prebuilt" branch is to avoid that, since it's not possible to cross-build for win32 from darwin).

In order to get the correct binaries when using my "prebuilt" branch, the best way to do it is to add an .npmrc file to your project that specifies the electron runtime as your target. That will cause the npm install command to install the correct version for electron on your current arch. This is an alternative to using electron-rebuild.

Here is my .npmrc from a project where I'm using robot-js prebuilt binaries:

# Tell NPM that this project uses the "electron@1.6.8" runtime, not the system "node" runtime.
# This will affect the binary-module versions that get fetched by "node-pre-gyp" when "npm install" is run.

runtime = "electron"
target = "1.6.8"

This will make things work correctly for local development (when running your project using the electron . command). As far as packaging your project, (especially cross-platform) it gets a little trickier. I'm using electron-builder to package my project, and I've added these scripts to my package.json in order to fetch the appropriate robot-js binary for each different target build (all of which I package on darwin):

    "dist:win": "npm install robot-js --target_platform=win32 --target_arch=x64 && node-pre-gyp install -C node_modules/robot-js --target_platform=win32 --target_arch=ia32 && build --win --ia32 --x64",
    "dist:mac": "npm install robot-js --target_platform=darwin --target_arch=x64 && build --mac --x64",
    "dist:linux": "npm install robot-js --target_platform=linux --target_arch=x64 && build --linux --x64"

The npm install robot-js ... command clears any current copy and installs the explicitly-specified binaries into node_modules, then the build command from electron-builder packages for the specified platform.

The windows platform in my case is a little special since electron-builder is capable of creating a dual-mode installer for both 64-bit and 32-bit, so I install the 64-bit binaries first using npm install (which also cleans my dev copy of the darwin binary), and then add the 32-bit binary using node-pre-gyp directly (which doesn't clear the previous binary) so I end up with both 64 and 32-bit binaries in my node_modules tree, and electron-builder packages both as I intended. (note: after cross-building, if I want to go back to local development, I need to run npm install robot-js again with no additional arguments to flip back to the darwin binary)

A perhaps easier way to do it if you don't mind unused additional binaries getting packaged into your distributable package would be to just install the binaries for all three environments at once. To do that, you can just add some calls to node-pre-gyp with the various archs in your install script. Something like this:

"install": "node-pre-gyp install -C node_modules/robot-js --target_platform=win32 --target_arch=ia32 && node-pre-gyp install -C node_modules/robot-js --target_platform=win32 --target_arch=x64 && node-pre-gyp install -C node_modules/robot-js --target_platform=linux --target_arch=ia32 && node-pre-gyp install -C node_modules/robot-js --target_platform=linux --target_arch=x64 && node-pre-gyp install -C node_modules/robot-js --target_platform=darwin --target_arch=x64"

If you do that, then electron-builder should just work, since it will grab all the binaries from your node_modules tree, which will now contain the ones needed for all possible platforms.

Also note: npm@5 is currently broken when using node-pre-gyp, so you'll want to stay on npm@4 until they work that out. There's a workaround necessary if you use npm@5 as noted here: mapbox/node-pre-gyp#298 (comment)

[karliky]

You guys rock my world, I just got it working 💯 👍 🥇

Here is what I did in case anyone needs help with this:

• Run npm init to create a new nodejs blank project
• Set your .npmrc, this is mine:

runtime = "electron"
target = "1.6.8"
target_arch = x64
disturl = https://atom.io/download/atom-shell
progress = true

• Add this to your package.json:

  "devDependencies": {
    "electron": "1.6.8",
    "robot-js": "git://github.com/p120ph37/robot-js.git#prebuilt"
  },
  "scripts": {
    "start": "electron ."
  },

• run npm install
• you should see something like this: [robot-js] Success: "C:\Users\WobCraft\Downloads\frida-master\robot\node_modules\robot-js\lib\win32-x64-electron-v1.6\robot.node" is installed via remote
• launch electron with npm start

Enjoy! :)

[dkrutsko]

You rock @karliky. I'm like the worst project manager in the world for being so far behind schedule. But if I can get what I'm working on to work, it'll more than make up for it! By the way, did any of you have a chance to test the dev build with Node 8? Does it actually compile?

[p120ph37]

I don't want to install node@8 on my dev machine yet (not a fan of nvm on mac), but I can add node@8 to the build for Travis/Appveyor and see if it implodes. One tricky part is that node-pre-gyp maintains a hard-coded node-version/ABI cross-reference, which as of the latest release, only includes 8.0.0, not 8.1.0, so it will fail to work on anything other than the original 8.0.0 preview. Their master branch does have some newer node versions (including 6.11.0 and 8.1.0), but I don't want to use unreleased code in my branch. They have been working in mapbox/node-pre-gyp#277 on a way to make the ABI lookup less of a manually-maintained thing, but haven't released that yet - when (if?) it does get released, it should simplify things.

[dkrutsko]

@p120ph37 Really? I'm a huge fan on NVM on Mac, Haven't had any problems. But for Node, if 8 works then 8.1 should work no problem since the V8 versions are compatible across major node releases. We'll see though, I'm hoping nothing broke since 7.

[p120ph37]

Yeah, the ABI from 8 to 8.1 should be the same, but node-pre-gyp doesn't know that in the last-released version... :-(

[p120ph37]

CI build is looking good so far. 8.0.0 built and passed the "types" test.

[p120ph37]

Merged to prebuilt, build finished, and binaries available for ABI 57 now.

[dkrutsko]

@p120ph37: Okay, I've been avoiding this project for far too long! But no more, I'm not doing anything else until I get this sorted out. First of all, I was quite impressed by your prebuilt branch and I'd like to import as much stuff from there as possible, including all the CI stuff. Second, I was looking at node-sass as they have a pretty good pipeline going (i.e. If no precompiled binaries were found, it auto-compiles them using node-gyp). So I'll work on getting everything set up and integrated on the dev branch.

While all this is happening, I'd like to also extend an invitation to both @p120ph37 and @karliky to join the robot project as official collaborators. Let me know if you're interested in this and I'll add you to the team! My overall goal is to work on releasing robot 2.1 and then possibly rewriting robot-js using ES6 and NaN. There's a lot of work ahead and I want to get serious about it.

[karliky]

Of course I want to contribute to robot as collaborator! 👍 🥇
I can help with ES6, unit tests, docs and anything you need help with. Thank you!

[dkrutsko]

@karliky Super! Welcome to the team, we should probably get a chatroom or Skype going rather than communicating exclusively through this PR ha ha. I have a Gitter set up which might be useful. But we could also maybe use Slack, Discord, etc.

[karliky]

@dkrutsko gitter is fine! Thank you!

[dkrutsko]

Okay I'm merging this into a temporary branch (issue-20), which will resolve #2, #4, #25, and #32 as well as implement a few other changes from the p120ph37/robot-js/prebuilt branch! Discussion continued in #35!