Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#15413 closed defect (fixed)

[patch][ccla] build enhancement: support node.js on windows (no cygwin)

Reported by: Patrick Ruzand Owned by: Patrick Ruzand
Priority: blocker Milestone: 1.9
Component: BuildSystem Version: 1.7.2
Keywords: Cc:
Blocked By: Blocking:

Description

The attached patch enables the build to be run on node.js under windows "natively" (that is, without cygwin installation). It consists on:

  • setting properly the has("is-windows") flag when node is hosting (main.js)
  • properly format the various path variables (javaClasses, etc) needed to run the java processes (writeOptimized.js)

Tested on win7 with node 0.6.18, on linux (ubuntu 12.04) with node 0.6.12

Attachments (4)

node-win.patch (2.2 KB) - added by Patrick Ruzand 7 years ago.
add node.js + windows support, patch by pruzand (IBM, CCLA)
node-win.2.patch (3.9 KB) - added by Patrick Ruzand 6 years ago.
patch updated following recent changes in trunk (IBM,CCLA)
node-win-1.8.patch (3.6 KB) - added by Patrick Ruzand 6 years ago.
patch for 1.8 branch with some typos fixed. (IBM, CCLA)
node-win.3.patch (3.9 KB) - added by Patrick Ruzand 6 years ago.
new patch for trunk, fix broken build under OSX (IBM,CCLA)

Download all attachments as: .zip

Change History (34)

Changed 7 years ago by Patrick Ruzand

Attachment: node-win.patch added

add node.js + windows support, patch by pruzand (IBM, CCLA)

comment:1 Changed 7 years ago by Rawld Gill

Milestone: tbd1.8
Priority: undecidedlow
Status: newassigned
Type: enhancementdefect

comment:2 Changed 7 years ago by Rawld Gill

@pruzand: I tried your patch and I couldn't get it to work. I get

CreateProcessW: The system cannot find the file specified.

when trying to spawn the java process. I tried several variations...full java filename w/ .exe, quotes, etc. and just couldn't get it to work. I'm sure this is some node-ism that we'll just have to play with to figure out. If you can get me something early on July 18th, I'll try to push this into 1.8 RC.

comment:3 Changed 7 years ago by Rawld Gill

Milestone: 1.82.0

comment:4 Changed 7 years ago by millennium

Could this patch make it to 1.8.1? Works like a charm and nod.js is way faster then rhino.

470 seconds vs 150 seconds

comment:5 Changed 7 years ago by Colin Snover

I would say the build system not working on Windows is more of a defect than an enhancement at this point and would not object to fixing it before a new major release.

comment:6 in reply to:  2 ; Changed 7 years ago by Patrick Ruzand

Hi rawld,

Replying to rcgill:

@pruzand: I tried your patch and I couldn't get it to work. I get

CreateProcessW: The system cannot find the file specified.

when trying to spawn the java process.

I have just tried the patch with node v0.8.11 on windows 7 64, and got no errors. The build has ended successfully. Here is the cmd line I used from a standard cmd.exe prompt (no cygwin on my system): node ..\..\dojo\dojo.js load=build --profile ..\..\..\demo.profile.js --release

Tell me if you need more info.

comment:7 in reply to:  6 Changed 7 years ago by iCanDo

Replying to pruzand:

I have just tried the patch with node v0.8.11 on windows 7 64, and got no errors. The build has ended successfully.

It works with node v0.8.11 on win 7 x64, but build is 3x times slower (java part).

comment:8 Changed 7 years ago by cjolif

Priority: lowhigh

Not being able to use Node.js to do my build on Windows is really problematic. There is a bunch of users out there that don't run MacOS or Linux. I don't think we can wait for 2.0 to get that addressed. I'm bumping the priority. Knowing there is a patch available can we get an agreement that this should at least make 1.9? Thanks.

comment:9 Changed 7 years ago by simpo

Thank you, been trying to build with Node all morning and could not figure-out how to get it working. This works perfect (so far) with node 0.8.12 and Windows XP.

comment:10 Changed 6 years ago by millennium

Upgraded today to dojo version 1.8.1 supprised to see this patch didn't made it yet to the source. Cause it only effects windows users in a positive way.

comment:11 Changed 6 years ago by cjolif

Milestone: 2.01.9

rawld do you have any objection Patrick committing this in trunk for 1.9? This would really be useful to Windows users.

comment:12 Changed 6 years ago by inghamc

I'm also wondering why Windows developers still can't custom build with NodeJS, when this patch works beautifully? Why shouldn't this be included in 1.8.2?

In our project I have to apply the patch as part of our Maven build before building Dojo; otherwise the ~10s custom build becomes ~90s with Java, which is just unacceptable in a client-side dev cycle.

Changed 6 years ago by Patrick Ruzand

Attachment: node-win.2.patch added

patch updated following recent changes in trunk (IBM,CCLA)

Changed 6 years ago by Patrick Ruzand

Attachment: node-win-1.8.patch added

patch for 1.8 branch with some typos fixed. (IBM, CCLA)

comment:13 Changed 6 years ago by Patrick Ruzand

Milestone: 1.91.8.2

comment:14 Changed 6 years ago by Patrick Ruzand

Rawld, it would be really great to have this patch in 1.8.2 for all our windows users. If you don't have the time, I can commit it. Let me know.

comment:15 Changed 6 years ago by Rawld Gill

In [30138]:

improve builder to work with native windows version of node.js; thanks pruzand; refs #15413; !strict

comment:16 Changed 6 years ago by Rawld Gill

Owner: changed from Rawld Gill to Colin Snover

The patch won't work with trunk owing to [30044].

comment:17 in reply to:  16 Changed 6 years ago by Patrick Ruzand

Replying to rcgill:

The patch won't work with trunk owing to [30044].

For trunk, I have attached an updated version of the patch to match #30044 changes. See node-win.2.patch.

Last edited 6 years ago by Patrick Ruzand (previous) (diff)

comment:18 Changed 6 years ago by bill

BTW, worked for me on trunk, using the command:

c:\trunk\util\buildscripts>node ../../dojo/dojo.js load=build --profile standard
 --release --releaseDir c:/release --optimize closure --layerOptimize closure

comment:19 Changed 6 years ago by cjolif

bill I suppose it would still be better to be able to use Windows-style paths on Windows?

comment:20 Changed 6 years ago by bill

For the releaseDir? I agree, and I tried that first, but it didn't work for me.

comment:21 Changed 6 years ago by Patrick Ruzand

Bill, in your example you don't give a path to your profile file. Specify a path (relative or absolute) and it will fail. And yes, better to stick to native path notation IMO.

comment:22 Changed 6 years ago by Patrick Ruzand

[for the log, discussed this with bill]
In fact, I misunderstood Bill's comment. So indeed, with the patch applied, specifying a path using windows syntax in the releaseDir cmd line argument fails.
It is not linked to this patch though, as it was already the case before, with rhino. My understanding is that all profile properties that deal with path (releaseDir, paths, packages location, etc) must be specify using the unix syntax.
Ideally, at least for the path-related command line properties, both syntaxes should be supported IMO. I will fill an enhancement ticket to track this one.

Changed 6 years ago by Patrick Ruzand

Attachment: node-win.3.patch added

new patch for trunk, fix broken build under OSX (IBM,CCLA)

comment:23 Changed 6 years ago by Kitson Kelly

Milestone: 1.8.21.9

Moving to 1.9 Milestone since most of the fix is in 1.8 branch now and now patch needs to be committed to trunk.

comment:24 Changed 6 years ago by Colin Snover

Priority: highblocker

If this fix does not land in trunk then it sounds like we’ll be regressing 1.9 so marking as blocker.

comment:25 Changed 6 years ago by Colin Snover

In [30826]:

Apply patch to trunk to fix Node.js builds on Windows. Thanks pruzand. Refs #15413. !strict due to invalid code

comment:26 Changed 6 years ago by Colin Snover

Owner: changed from Colin Snover to Patrick Ruzand

Could you please build trunk on Windows one last time just to make sure that this is working correctly? Thanks.

comment:27 Changed 6 years ago by Patrick Ruzand

I did a test with the buildscripts/profiles/amd and baseplus profiles. It works.

comment:28 Changed 6 years ago by bill

Resolution: fixed
Status: assignedclosed

comment:29 in reply to:  28 ; Changed 6 years ago by trizer

Replying to bill:

Hi, I tried running the build with the latest code (as of this post) and the build fails without any informative error messages. I haven't changed any of the profile files since dojo v1.8 and I run on Node v0.8.11 and Windows 7 with msys shell. Is there a way to turn on some more informative debugging messages or ways I can debug this?

Below is the only thing I see even with the node --debug flag.

.
.
.
starting parsing resource...
Copy & minify index.html to dist
./build.sh: line 58: /path/to/dist/index.html: No such file or directory

Thanks

comment:30 in reply to:  29 Changed 6 years ago by Patrick Ruzand

Replying to trizer:

Hi, I tried running the build with the latest code (as of this post) and the build fails without any informative error messages. I haven't changed any of the profile files since dojo v1.8 and I run on Node v0.8.11 and Windows 7 with msys shell. Is there a way to turn on some more informative debugging messages or ways I can debug this?

It seems you run the build.sh in a msys shell. I am not sure the unix shells windows ports (cygwin, msys, etc) are (or have been) supported (to be confirmed).
But the purpose of this patch is to make node.js build working on windows within a true windows shell (ie. using windows command prompt).

  c:\work\dojo\util\buildscripts>node ..\..\dojo\dojo.js load=build --profile path\to\profile.js
Note: See TracTickets for help on using tickets.