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

jake.exec: don't wrap DOS command in double-quotes #100

Merged
merged 1 commit into from
Feb 19, 2012

Conversation

mpareja
Copy link
Contributor

@mpareja mpareja commented Feb 19, 2012

Wrapping in double-quotes is not required by cmd.exe and actually
prevents the command from being executed. For some reason, when the
command is actually executed, the double-quotes are escaped with a
preceding backslash.

Note: I only tested this on an XP virtual machine running on Ubuntu.

Wrapping in double-quotes is not required by cmd.exe and actually
prevents the command from being executed. For some reason, when the
command is actually executed, the double-quotes are escaped with a
preceding backslash.
mde added a commit that referenced this pull request Feb 19, 2012
jake.exec: don't wrap DOS command in double-quotes
@mde mde merged commit 32383be into jakejs:master Feb 19, 2012
@mde
Copy link
Contributor

mde commented Feb 19, 2012

Merged, thanks.

@vvs
Copy link

vvs commented Feb 27, 2012

Ah, thank you for the fix! Essentially, without it jake on Windows is totally hosed, impossible to launch any external commands. It would be nice to have an updated version released for those poor souls who develop node.js apps on Windows.

@mpareja
Copy link
Contributor Author

mpareja commented Feb 27, 2012

@vvs No worries! I'm starting to work on a some tools for building .NET apps using jake. Hopefully, I can help fix any other Windows specific issues I find along the way.

@prabirshrestha
Copy link
Contributor

@mpareja I have been working with some .net helpers tools. i haven't yet pushed everything online yet though. https://github.com/nJake

Here is the sample https://gist.github.com/1182130

I could share you some ideas I have got.

@mde
Copy link
Contributor

mde commented Feb 27, 2012

Just pushed a new release. This fix is in v0.2.19.

@mpareja
Copy link
Contributor Author

mpareja commented Feb 27, 2012

@prabirshrestha It would be good to mash some of our ideas together. It would be especially useful to share some of the tasks themselves.

At the moment, I'm taking the stance that I don't want paprika to require jake. I may, in the future, release a side-package tailored for working with jake, but I'm not sure that is necessary. I took a quick look at your gist. I'm not convinced that having different methods of defining jake tasks is a good idea. At the moment I'm going with this approach:

var msbuild = require('paprika').msbuild;
task('build', function () {
    msbuild({
        file: '../MySolution.sln'
      , version: 'net35'
      , processor: 'x86'
      , targets: ['Clean', 'Build']
      , properties: { Configuration: 'Release' }
      , show_stdout: false
    });
};

What do you think?

@prabirshrestha
Copy link
Contributor

I'm pretty open to the api. This is actually not the main problem. (Your api seems to be much better then mine.)

I have been looking at the best way to solve problems that albacore faces. (Rake tasks for .net https://github.com/derickbailey/Albacore).

What I would like is a official way for anyone to write and distribute custom tasks/modules like msbuild. Jake could then have a wiki page referring to the npm packages. It also doesn't make sense for msbuild task to be distributed in the core jake as some of them may not even be using windows and having tasks such as msubild is also a maintenance headache for @mde when more tasks/modules are created.

@mpareja I would also like to have a default settings so that I don't need to set the version everytime.

var msbuild = require('paprika').msbuild;
msbuild.config({ version: 'net35', processor: 'x86', properties: { Configuration: 'Release' } });

task('build', function () {
    msbuild({
        file: '../MySolution.sln'
      , targets: ['Clean', 'Build']
      , show_stdout: false
    });
};

What we need is a standard way to write/consume jake tasks. So everyone whether a nodejs/java/.net/c++ all can benefit.

I would also sugest adding two options. _command and _parameters.

msbuild({
    _command: 'c:\mono\xbuild.exe',
    _parameters: '/nologo /version'
});

_command and _parameters are additional special options. having _command will allow be to easily use xbuild on mac/linux by compiling with mono. _parameters also makes it easier to support future options or options that are currently not exposed via the msbuild task.

@mpareja
Copy link
Contributor Author

mpareja commented Feb 27, 2012

@prabirshrestha - I really like your suggestions for adding _command and _parameters. I'll probably create an issue in paprika. I'll also take a look at adding configurable default settings.

Regarding Albacore: I am planning on taking inspiration from it with regards to what tasks to build (no pun intended) next. Were there some other issues you've seen @derickbailey face with Albacore?

I agree, we need some guidance for publishing reusable tasks(?) for use with jake. Oddly enough, it was through struggling to come up with an API that was inline with the 'jake' way, that I decided not to actually tie paprika to jake. I can always add a 'jake' specific module that leans on paprika in the future. I did look at some of the tasks that come with jake, but I'm not sure I like the constructor approach...

@prabirshrestha
Copy link
Contributor

another option that jake needs would be unified logging system and verbosity mode.

This would allow tasks such as msbuild to display the full command path on how it executed msbuild depending on verbosity mode.

@mde
Copy link
Contributor

mde commented Feb 29, 2012

Added an issue for unified logging, verbose-mode: #102

@prabirshrestha
Copy link
Contributor

In the end I landed up creating my own jake helper file for .net related tasks based on @mpareja (https://github.com/mpareja/paprika) syntax

Rather then distributing it as a npm package it is distributed as a single file. http://bit.ly/njakejs

This would be the easiest way to install njake. (This would avoid the use of node_modules folder totally.)

curl -Lk bit.ly/njakejs -o src/njake.js

One downloaded it can be committed to the source control so no package.json file or npm install is required to install njake.

For those that are interested you can find the source code of njake.js at https://github.com/prabirshrestha/njake

Facebook C# SDK now uses jake and njake as the build script which can be found at https://github.com/facebook-csharp-sdk/facebook-csharp-sdk/blob/5bbc18884ca92afae0ed238df874b26b638ca06a/jakefile.js

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

Successfully merging this pull request may close these issues.

4 participants