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 17:23:16 GMT
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>

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