You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by daserge <gi...@git.apache.org> on 2015/06/04 13:15:57 UTC
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
GitHub user daserge opened a pull request:
https://github.com/apache/cordova-ios/pull/144
CB-9116 Ensure platform scripts are executable as the platform could be added on non-OSX environment
Corresponding [Jira issue](https://issues.apache.org/jira/browse/CB-9116)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-ios CB-9116
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-ios/pull/144.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #144
----
commit 53c8b3ad2d64466b72c24e44b33d7dc648723520
Author: daserge <da...@yandex.ru>
Date: 2015-06-04T08:48:59Z
CB-9116 Ensure platform scripts are executable as the platform could be added on non-OSX environment
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-ios/pull/144#discussion_r31741687
--- Diff: bin/templates/scripts/cordova/lib/build.js ---
@@ -87,6 +87,13 @@ module.exports.run = function (argv) {
}
return Q.nfcall(fs.writeFile, path.join(__dirname, '..', 'build-extras.xcconfig'), extraConfig, 'utf-8');
}).then(function () {
+ // Ensure platform scripts are executable as the platform could be added on non-OSX environment
+ shell.find(projectPath).filter(function (file) {
--- End diff --
sounds good
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-108911479
LGTM !
- what causes file permissions to change when the platform is added on a non-OSX system ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by sgrebnov <gi...@git.apache.org>.
Github user sgrebnov commented on a diff in the pull request:
https://github.com/apache/cordova-ios/pull/144#discussion_r31713805
--- Diff: bin/templates/scripts/cordova/lib/build.js ---
@@ -87,6 +87,13 @@ module.exports.run = function (argv) {
}
return Q.nfcall(fs.writeFile, path.join(__dirname, '..', 'build-extras.xcconfig'), extraConfig, 'utf-8');
}).then(function () {
+ // Ensure platform scripts are executable as the platform could be added on non-OSX environment
+ shell.find(projectPath).filter(function (file) {
--- End diff --
Should we check `cordova` folder only? (projectPath/cordova). My concern is that there could be too many files in other folders like `www` so this could be slow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-110401243
Closing this - decided to rewrite the scripts in node.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-109396714
It'd be a great idea to convert the scripts from 'shell' to 'node'.
bash is hard to follow and make sense of.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-108975191
@omefire I don't have Android environment on Mac now, but as far as I can see there are no shell scripts in Android platform.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-109396260
Does it make sense to convert any of these scripts to node instead of 'shell'?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on a diff in the pull request:
https://github.com/apache/cordova-ios/pull/144#discussion_r31719008
--- Diff: bin/templates/scripts/cordova/lib/build.js ---
@@ -87,6 +87,13 @@ module.exports.run = function (argv) {
}
return Q.nfcall(fs.writeFile, path.join(__dirname, '..', 'build-extras.xcconfig'), extraConfig, 'utf-8');
}).then(function () {
+ // Ensure platform scripts are executable as the platform could be added on non-OSX environment
+ shell.find(projectPath).filter(function (file) {
--- End diff --
Makes sense; although there is another script there:
`ios/build/HelloCordova.build/Debug-iphonesimulator/HelloCordova.build/Script-304B58A110DAC018002A0835.sh`
So we can probably check all folders except `www` - what do you think?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by daserge <gi...@git.apache.org>.
Github user daserge closed the pull request at:
https://github.com/apache/cordova-ios/pull/144
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by daserge <gi...@git.apache.org>.
Github user daserge commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-108915724
@omefire I think that it is because git does not include exec permission for these files.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-ios pull request: CB-9116 Ensure platform scripts are exec...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-ios/pull/144#issuecomment-108966698
@daserge , does this behavior show up if for example, I add android from windows, then check it into git ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org