-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
Windows support on the bash/sh code #22
Comments
Could you provide a little more context? What maid file were you trying to run? |
I think this will involve a few things. First, is support for powershell and cmd. That's probably the easiest thing. The next consideration: Should there be a fallback. i.e. a bash script for unix systems and a ps or cmd script for windows. Maybe we can just add a platform support section to checkTypes and have it automatically skip things it doesn't support. |
+1 |
Such issues are not very helpful. 😕 Please provide example maidfile and what you run, @AddictArts & @daniel100097 ... |
@olstenlarck Here you go ## out
```sh
yarn outdated —depth=0
Pretty much the simplest task will fail. It seems like there is a lack of Windows machines used to test on. I think the error clearly shows “sh” or “bash” does not exist to execute or spawn. |
Yep, exactly that. We need to check to ensure the script execution language exists before running it... question is, how do we fail? We probably need a way to denote that a block only runs on a certain platform. |
@AddictArts then just not use Probably good thing would be @daniel100097 or @egoist to change the title of this issue. We are talking here for windows support on the @AddictArts does other "languages" work? e.g. (offtopic: how strage is user with debian-like logo avatar to ask for Windows ;d) |
@olstenlarck it would be great to use command or powershell, but maid doesn't support either, look at the code, checkTypes ... ['sh', 'bash'] ... ['py', 'python'] ... ['js', 'javascript']. That is it. I would suggest that "sh" work for the "CMD" prompt, but not "bash", unless support for something like mingw32/64 bash terminal was desired. The reason is so that the maid task can run on multi os. Yes I understand that not all commands in an sh task will work, but things like simple echo's would and maybe some others. @olstenlarck Yes 'js' works |
Oh yea, really.. the checkTypes thing. Okey how we can fix that? With another fence name? Or it is just a problem how the edit: ahhhh yeah... definitely. If we switch to |
@AddictArts can you please try with the following const path = require('path')
const execa = require('execa')
const MaidError = require('./MaidError')
module.exports = ({ task, type = task.type, resolve, reject }) => {
const modulesBin = path.resolve(path.join('node_modules', '.bin'))
const promise = execa(type, [task.script, ...process.argv.slice(2)], {
stdio: 'inherit',
env: Object.assign({}, process.env, {
PATH: `${modulesBin}:${process.env.PATH}`
})
})
promise.on('close', code => {
if (code === 0) {
resolve()
} else {
reject(new MaidError(`task "${task.name}" exited with code ${code}`))
}
})
// we don't need to return
// 1. because this function is executed in new Promise(() => {})
// 2. we have its resolve and reject, so it's better to use them
promise.then(resolve).catch(reject)
} I definitely believe that it would work. edit: @egoist probably add AppVeyor? |
@olstenlarck thanks, but I didn't have time to fork and try the patch. Exca is an additional dependency if that matters. Also exca just uses cross-spawn, so maybe just that? This did help me learn the issues around spawn and Windows, here are some relevant bits, including cross-spawn above. This issue can be closed, I've abandoned maid for now, really needed it to work out of the box to see how it could be useful. Thanks for the help, suggestions, and comments. |
If it was "just that" it would not exist.
You don't need to waste time. And yea, even i still can't figure out how to fix it, that's why i paused the work on #40 and that's why it fails. We also need AppVeyor, otherwise it would be just hard to us to continue work on such support. |
@olstenlarck just to be clear, I was inputting that only cross-spawn could be considered versus using exca. Use whatever you want though and the exca project is certainly more than cross-spawn. I'm not understanding the "waste time" comment; However, for me it was not a waist of time and I hope this helps the project and someone else like it helped me. |
Yea, totally. Sorry, don't take me hard, i forgot the emojis :D |
I created a very similar tool, called saku, which only supports shell for task langauge, but works in windows as well. |
`events.js:182
throw er; // Unhandled 'error' event
^
Error: spawn bash ENOENT
`
The text was updated successfully, but these errors were encountered: