-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Framework: use npm scripts instead of make for build system #3520
Comments
@Pomax Thank you for the issue. We are aware of the additional complexity of running wp-calypso on windows. It has been pointed out in a number of other issues (#592, #1193, #2029, #2066). There's some discussion in #1193 about why we had originally chosen make for our build system in that issue. The choice is a bit of an artifact from two years ago when calypso was still an experimental internal project with just a handful of developers. I think you're right that npm scripts would be an improvement. I will change the title of your issue to more simply state your request. We don't have anyone actively working on this right now, but of course pull requests are welcome if you're eager. In the meantime, I would suggest you take a look at #2124 where we have put some effort into making our current Makefile system installable and runnable on windows. If the instructions in windows.md do not work for you, you could comment on that PR or open a new issue and ping @klimeryk with questions. Or you may want to try using our wp-calypso-bootstrap project which uses |
Unfortunately those instructions only work by violating the basic concept of "where programs install" on windows, by telling people to install things on the drive root: something that UAC normally forbids (the system drive root is a protected location, programs go in "program files" and "program files (x86)"). I'll have a look at how much work it is to turn your makefile into npm scripts, because make was finally on its way out 2 years ago, and seeing it still in use for a modern Node-based project is really sadmaking, especially since node+npm are unquestionably properly-cross platform, where make is horribly deficient in that respect =) (also, I'm using a VM solution at the moment. Calypso is rather intensive, and even on an i7 4790 with 32GB of ram it runs quite slowly. It's a solution, but not one to be used for too long) |
@Pomax thank you for your comments and suggestions. I completely agree that
I wouldn't call a project "unquestionably properly cross-platform" that has a huuuuuuuge issue titled "Windows users are not happy" for Also, please don't lump every problem on |
LOL sorry, I tried to write it in a way to make clear that I was cross-linking for reference, not to say that it's a make problem per se. Clearly I failed in that. :D |
@folletto - no problem, I wasn't actually addressing that at you 😃 It's just that @Pomax's comments suggest that every problem we have with regards to our build would be solved by switching to npm scripts - which I believe is not true. I used the "folders with spaces" argument just as an example of blaming everything on |
I'm not blaming everything on make, it's indeed still used a lot for compling unix/linux programs, but for projects that don't need compilation, it was definitely on the way out. Since this seems a predominantly node codebase, a lot of the issues around getting calypso installed on windows seemed to be because If there are other places in the code itself that can't actually run when there are spaces in paths, then that is unfortunate, and hopefully all of those places are known and files as issues to tackle in the future. I'm going to see if I can find some time to turn as much of the make script into npm management instead, and if that works I'll be happy to file a PR. |
Just wanted to mention that it might be caused by synced folders. We moved away from using VirtualBox shared folders syncing mechanism in Calypso Bootstrap because it was killing performance. |
Unfortunately, I'm already using VMware rather than VirtualBox for that very reason (there's a longstanding bug around syncing that breaks synced folders for windows users because Oracle's not updated its win calls to use unicode paths, so you're stuck with 255 instead of 32000+ characters for a path. Which is pretty disastrous for OSS given that a lot of potential contributors tend to be on windows machines) |
I've filed a PR with refactors that moves some things from the makefile into package.json as npm scripts, such that a |
@Pomax Perhaps ShellJS could help with this? From: https://github.com/shelljs/shelljs See also: http://blog.npmjs.org/post/118810260230/building-a-simple-command-line-tool-with-npm |
feel free to also file a PR that uses ShellJS instead. In the mean time I've filed one that works quite well for windows users, without adversely affecting 'nix users, while also setting up a path to move away from the makefile approach in small steps, for universal cross-platform support at some future point in time. |
@Pomax thank you very much for submitting the PR. I'm not sure I'll get a chance to look today myself, but I'll try to get some eyes on it. |
With #4596 we fully migrated our test runner to npm scripts. You can run tests nows with |
oh nice, great job! |
Fixed by #13536. |
FYI, PR to remove |
Steps to reproduce
What I expected
git clone
npm install
npm start
Like any npm-managed project. Nice and OS agnostic, guaranteed to work on unix, linus, OSX, windows, and pretty much anything else that lets you install node and npm, with all bootstrapping managed via npm scripts triggered by the npm "postinstall" hook.
What happened instead
The docs tell me to use
make
, something that is notoriously unreliable and absolutely not usable cross-platform. To make matters worse, a windows user is doubly-impacted, because they are also instructed to install MSYS2, a tool chain that breaks when there are spaces in paths, but also tell them to use the windows git client which installs in the "program files (x86)" folder, with spaces. The instructions are literally set up such that windows users can't install Calypso unless they go out on their own and figure out how to get "make" to work.This is super-silly, just turn that makefile into a set of npm scripts instead. If you've not heard of npm scripts, or you only have passing knowledge of them: npm scripts are scripts that run as CLI commands, just like MAKE instructions, except they run with execution access to any and all CLI utilities installed in the
node_modules
dir, which means that instead of unix-only commands likerm
orcp
, you can use universal packages like rimraf or cp for node and be guaranteed that no additional tool chains are needed: things just work. (Need environment variables? don't use ENV on the command line, use an .env file and habitat)This will work, for instance:
It's 2016, we have a universally supported system installed already (node+npm), there is no need for really-only-unixy-systems
make
anymore. Everyone wins.The text was updated successfully, but these errors were encountered: