The topic discussing standalone drush scripts should probably clarify that they only work on Windows. The test for them should skip if test is run on Windows ... Unless I am missing something and the shebang should work on Windows.

We might want to include or copy a couple functions into the test framework. Am thinking of drush_is_windows() and friends.

Comments

greg.1.anderson’s picture

The shebang scripts will probably work on Windows under mingw / cygwin, but you are right, the test should be skipped on other platforms.

greg.1.anderson’s picture

Status: Active » Needs review

Here is a test for bash prior to testing the shebang script. We just naively assume that bash is present on Linux (Mac) and Cygwin (MINGW), and not on Windows, rather than do 'which bash' or equiv. on every bootstrap. The test is superfluous, in a way, because without bash, we will never be called in a way that activates this code; however, it does serve as documentation of sorts what the intention is. I don't know, maybe that makes the test potentially harmful? (If we're wrong in drush_has_bash, we turn off behavior for no good reason.)

Note that with or without this patch, you can run shebang scripts via drush php-script c:/path/to/script on both Linux and Windows.

moshe weitzman’s picture

Status: Needs review » Needs work

Missing the attachment

greg.1.anderson’s picture

Status: Needs work » Needs review
StatusFileSize
new3.64 KB

Oops. Here it is.

greg.1.anderson’s picture

Issue tags: +Windows

Add windows tag

moshe weitzman’s picture

Status: Needs review » Needs work

Seems like a useful enhancement. Lets enhance the patch so that we invoke the hellounish script with `drush script hellounish.script` when running on windows. Am referring to coreTest.php

beltofte’s picture

StatusFileSize
new3.66 KB

Updated the patch to current HEAD.

@moshe: Not 100% sure how the test case should be altered here. Let's discuss it on our Skype meeting later today.

greg.1.anderson’s picture

So, about the line elseif (((strpos($args[1], ' ') !== FALSE) || (!ctype_alnum($args[1][0]))) && (_drush_is_drush_shebang_script($args[2])))...

This allows folks to pass arguments or options to drush on the shebang line, but it only works if you use the absolute path to drush; it cannot be made to work with #!/usr/bin/env drush. I don't think anyone is using this feature, and calling drush_set_option at the beginning of your script is a fine alternative, so I think this code block could be dropped with no ill effects.

moshe weitzman’s picture

@greg - can you clarify what patch you are envisoning here. not sure i can parse what you mean.

greg.1.anderson’s picture

I investigated #8 further and discovered that I was WRONG, that code is necessary for all shebang scripts.

moshe weitzman’s picture

Status: Needs work » Reviewed & tested by the community

Thanks. So #7 looks fine to me. Any objection?

greg.1.anderson’s picture

Works on Linux; I'm fine with committing it.

moshe weitzman’s picture

Status: Reviewed & tested by the community » Fixed

committed to master

Automatically closed -- issue fixed for 2 weeks with no activity.