cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carlos Santana <csantan...@gmail.com>
Subject Re: Plugman engine check and WP7/8
Date Wed, 23 Oct 2013 18:23:08 GMT
Actually just try it out and see that using spawn with "cmd"
["/c","cordova/build",...
is better option than adding the ".bat" then it covers "build.exe",
"build.bat", and "build.cmd" on windows

if someone thinks this is bad route, please shime in this jira issue [1]

[1]: https://issues.apache.org/jira/browse/CB-5187

--Carlos



On Wed, Oct 23, 2013 at 1:23 PM, Carlos Santana <csantana23@gmail.com>wrote:

> Thanks Bryan for pointing out;
> I created a jira issue to address the "compile, emualate, run" using spawn
> and not addressing windows ".bat"
>
> https://issues.apache.org/jira/browse/CB-5187
>
> I'm guessing the fix is to detect that node environment is Windows and
> append ".bat" to cmd for child process spawn
> We can't use exec, that was the original problem why we moved to spawn per
> Braden suggestion about stdio
>
> Not sure what to say about the version topic being discuss, maybe someone
> with knowledge about the problem should open a jira issue
>
> I'm out on a 2 day conference, will try to land something today unless
> wants to take over and help.
>
>
> --Carlos
>
>
>
> On Wed, Oct 23, 2013 at 11:06 AM, Bryan Higgins <bryan@bryanhiggins.net>wrote:
>
>> I know Carlos just changed several CLI commands to use spawn. There
>> doesn't
>> seem to be any specific handling for Windows.
>>
>>
>> https://git-wip-us.apache.org/repos/asf?p=cordova-cli.git;a=commit;h=01c7ecec7ccf4a3c1423ddf3844e125d24965025
>>
>>
>> On Wed, Oct 23, 2013 at 10:44 AM, Braden Shepherdson <braden@chromium.org
>> >wrote:
>>
>> > I thought we were shelling out that command, not trying to load the
>> file.
>> > Then Windows would locate the version.bat file and run it, while Unixy
>> > platforms would see the version file with +x, and run it.
>> >
>> > Are we no longer going through the shell? (This should be using
>> > child_process.exec, not .spawn, for example.)
>> >
>> > Braden
>> >
>> >
>> > On Wed, Oct 23, 2013 at 10:13 AM, Bryan Higgins <
>> bhiggins@blackberry.com
>> > >wrote:
>> >
>> > > This issue has to do with the host system rather than platform.
>> Android
>> > and
>> > > BB10 both have version.bat files.
>> > >
>> > >
>> > > On Wed, Oct 23, 2013 at 9:56 AM, Sergey Grebnov (Akvelon) <
>> > > v-segreb@microsoft.com> wrote:
>> > >
>> > > > Hi,
>> > > >
>> > > > #1 The problem
>> > > > Right now the simplest (and also the most correct IMO) way to
>> specify
>> > > > plugin restrictions to specific cordova version is the following:
>> > > >
>> > > > plugin.xml:
>> > > >
>> > > >  <engines>
>> > > >    <engine name="cordova" version=">=2.7.0" />
>> > > >  </engines>
>> > > >
>> > > > But in this case as per plugman engines definition
>> > > > (plugman/src/util/default-engines.js) plugman will always try to
>> find
>> > > > version verification script in some predefined location, not taking
>> > into
>> > > > account currently running platform, and will fail on WP since
>> correct
>> > > > script name is version.bat, not just version.
>> > > >
>> > > > module.exports = function(project_dir){
>> > > >     return {
>> > > >         'cordova':
>> > > >             { 'platform':'*', 'scriptSrc':
>> > > > path.join(project_dir,'cordova','version') }, <- works in general,
>> but
>> > > NOT
>> > > > for WP7/8
>> > > >          ...
>> > > >         'cordova-wp8':
>> > > >             { 'platform':'wp8', 'scriptSrc':
>> > > > path.join(project_dir,'cordova','version.bat') }, <- correct
>> location,
>> > > not
>> > > > used in case of example above
>> > > >
>> > > >
>> > > > This means that right now there is no way to specify platform
>> dependent
>> > > > location of version verification script for 'cordova' engine check
>> > which
>> > > >  is going to be the most popular.
>> > > >
>> > > > #2 Proposed solution
>> > > >
>> > > > Taking into account we have platform context when we are looking for
>> > the
>> > > > appropriate engine
>> > > > plugman/src/install.js
>> > > >         function getEngines(pluginElement, platform, project_dir,
>> > > > plugin_dir){
>> > > >
>> > > > I propose to think about 'cordova' engine settings (in
>> > > default-engines.js)
>> > > > as a fallback in case we don't have any platform specific engine for
>> > some
>> > > > platform. So in case of  engine.attrib["name"] == 'cordova' we
>> should
>> > > check
>> > > > if there is engine with ['cordova-' + platform] name first and if
it
>> > does
>> > > > not exist use 'cordova' engine settings only.  For example we
>> already
>> > do
>> > > > this by the following line, but we need both engines (cordova and
>> > > > cordova-wp8) specified in plugin.xml file. Thoughts?
>> > > >
>> > > > // make sure we check for platform req's and not just cordova reqs
>> > > >     if(cordovaEngineIndex && cordovaPlatformEngineIndex)
>> > > > uncheckedEngines.pop(cordovaEngineIndex);
>> > > >
>> > > > PS. Another minor potential issue seems to be in check above;
>> > > > cordovaEngineIndex && cordovaPlatformEngineIndex will return
false
>> if
>> > one
>> > > > of indexes is zero (zero is valid/correct index in this context so
>> it
>> > > must
>> > > > be true).
>> > > >
>> > > > Thx!
>> > > > Sergey
>> > > >
>> > >
>> >
>>
>
>
>
> --
> Carlos Santana
> <csantana23@gmail.com>
>



-- 
Carlos Santana
<csantana23@gmail.com>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message