You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Bryan Higgins <bh...@blackberry.com> on 2013/10/23 16:13:21 UTC
Re: Plugman engine check and WP7/8
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
>
RE: Plugman engine check and WP7/8
Posted by "Sergey Grebnov (Akvelon)" <v-...@microsoft.com>.
Jira issue + fix
https://issues.apache.org/jira/browse/CB-5192
https://github.com/apache/cordova-plugman/pull/29
Thx!
Sergey
-----Original Message-----
From: Tim Kim [mailto:timkim85@gmail.com]
Sent: Thursday, October 24, 2013 12:48 AM
To: dev@cordova.apache.org
Subject: Re: Plugman engine check and WP7/8
Howdy all,
I like Carlos' suggestions in this jira ticket for dealing with Windows
scripts:https://issues.apache.org/jira/browse/CB-5187
Appending a .bat or using cmd /c seem pretty easy fixes in this case. I'm
+1 either way.
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?
The reason for having both cordova and cordova-platform in your plugin.xml engine definition is so that you can override the base cordova version if a platform needs to have a different version.
eg,
<engines>
<engine name="cordova" version=">=2.7.0" /> // this plugin will work on all platforms >=2.7.0
<engine name="cordova-android" version=">=3.0.0" /> // except for android which needs to be >= 3.0.0 </engines>
// 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).
Good catch. The getEngines function could use a refactor.
Are you ok if I create ticket for this specific bug (plugman + wp78) and
> fix it as per steps 1 and 2 above?
I say go for it.
--
Timothy Kim
On 23 October 2013 12:51, Josh Soref <js...@blackberry.com> wrote:
> On 10/23/13 2:23 PM, "Carlos Santana" <cs...@gmail.com> wrote:
> >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]
>
> I'm actually +1 on this approachŠ
>
> [Note: I've become the recognized Windows expert here in under a week]
>
> ---------------------------------------------------------------------
> This transmission (including any attachments) may contain confidential
> information, privileged material (including material protected by the
> solicitor-client or other applicable privileges), or constitute non-public
> information. Any use of this information by anyone other than the intended
> recipient is prohibited. If you have received this transmission in error,
> please immediately reply to the sender and delete this information from
> your system. Use, dissemination, distribution, or reproduction of this
> transmission by unintended recipients is not authorized and may be unlawful.
>
>
--
Timothy Kim
Re: Plugman engine check and WP7/8
Posted by Tim Kim <ti...@gmail.com>.
Howdy all,
I like Carlos' suggestions in this jira ticket for dealing with Windows
scripts:https://issues.apache.org/jira/browse/CB-5187
Appending a .bat or using cmd /c seem pretty easy fixes in this case. I'm
+1 either way.
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?
The reason for having both cordova and cordova-platform in your plugin.xml
engine definition is so that you can override the base cordova version if a
platform needs to have a different version.
eg,
<engines>
<engine name="cordova" version=">=2.7.0" /> // this plugin will work on
all platforms >=2.7.0
<engine name="cordova-android" version=">=3.0.0" /> // except for
android which needs to be >= 3.0.0
</engines>
// 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).
Good catch. The getEngines function could use a refactor.
Are you ok if I create ticket for this specific bug (plugman + wp78) and
> fix it as per steps 1 and 2 above?
I say go for it.
--
Timothy Kim
On 23 October 2013 12:51, Josh Soref <js...@blackberry.com> wrote:
> On 10/23/13 2:23 PM, "Carlos Santana" <cs...@gmail.com> wrote:
> >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]
>
> I'm actually +1 on this approachŠ
>
> [Note: I've become the recognized Windows expert here in under a week]
>
> ---------------------------------------------------------------------
> This transmission (including any attachments) may contain confidential
> information, privileged material (including material protected by the
> solicitor-client or other applicable privileges), or constitute non-public
> information. Any use of this information by anyone other than the intended
> recipient is prohibited. If you have received this transmission in error,
> please immediately reply to the sender and delete this information from
> your system. Use, dissemination, distribution, or reproduction of this
> transmission by unintended recipients is not authorized and may be unlawful.
>
>
--
Timothy Kim
RE: Plugman engine check and WP7/8
Posted by "Sergey Grebnov (Akvelon)" <v-...@microsoft.com>.
So I suppose we should
1. remove .bat extension from other script locations to be consistent, for example
'cordova-wp8':
{ 'platform':'wp8', 'scriptSrc': path.join(project_dir,'cordova','version.bat') },
2. Remove check for the file location on Windows before executing it due to additional different file extensions (we don't know exact target name, but .exec or .spawn cmd /c will be able to run it)
if(fs.existsSync(engine.scriptSrc)){
fs.chmodSync(engine.scriptSrc, '755');
var d = Q.defer();
child_process.exec(engine.scriptSrc, function(error, stdout, stderr) {
3. If we are switching to spawn we may want to wrap spawn functionality to special module which will hide all platform specific checks/magic
+ 1 for cmd /c due to same reason
Are you ok if I create ticket for this specific bug (plugman + wp78) and fix it as per steps 1 and 2 above?
Thx!
Sergey
-----Original Message-----
From: Carlos Santana [mailto:csantana23@gmail.com]
Sent: Wednesday, October 23, 2013 10:23 PM
To: dev@cordova.apache.org
Subject: Re: Plugman engine check and WP7/8
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 <cs...@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 <br...@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
> <cs...@gmail.com>
>
--
Carlos Santana
<cs...@gmail.com>
Re: Plugman engine check and WP7/8
Posted by Josh Soref <js...@blackberry.com>.
On 10/23/13 2:23 PM, "Carlos Santana" <cs...@gmail.com> wrote:
>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]
I'm actually +1 on this approachŠ
[Note: I've become the recognized Windows expert here in under a week]
---------------------------------------------------------------------
This transmission (including any attachments) may contain confidential information, privileged material (including material protected by the solicitor-client or other applicable privileges), or constitute non-public information. Any use of this information by anyone other than the intended recipient is prohibited. If you have received this transmission in error, please immediately reply to the sender and delete this information from your system. Use, dissemination, distribution, or reproduction of this transmission by unintended recipients is not authorized and may be unlawful.
Re: Plugman engine check and WP7/8
Posted by Carlos Santana <cs...@gmail.com>.
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 <cs...@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 <br...@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
> <cs...@gmail.com>
>
--
Carlos Santana
<cs...@gmail.com>
Re: Plugman engine check and WP7/8
Posted by Carlos Santana <cs...@gmail.com>.
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 <br...@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
<cs...@gmail.com>
Re: Plugman engine check and WP7/8
Posted by Bryan Higgins <br...@bryanhiggins.net>.
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 <br...@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
> > >
> >
>
Re: Plugman engine check and WP7/8
Posted by Braden Shepherdson <br...@chromium.org>.
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 <bh...@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
> >
>