Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-24943] CLI: Install command patch for Windows platform. #9198

Merged
merged 3 commits into from Jul 6, 2017

Conversation

ypbnv
Copy link
Contributor

@ypbnv ypbnv commented Jul 6, 2017

JIRA: https://jira.appcelerator.org/browse/TIMOB-24943

Description:
Installing a built from sources SDK fails on Windows.

Here I am not sure if stripping the 'unizp' command would work fine on OS X.

@ypbnv ypbnv added the bug label Jul 6, 2017
@ypbnv ypbnv requested a review from cb1kenobi July 6, 2017 13:31
@ypbnv ypbnv changed the title Install command patch for Windows platform. [TIMOB-24943] CLI: Install command patch for Windows platform. Jul 6, 2017
@@ -38,7 +38,7 @@ function install(versionTag, next) {

// TODO Combine with unzip method in packager.js?
// TODO Support unzipping on windows
exec('/usr/bin/unzip -q -o -d "' + dest + '" "' + zipfile + '"', function (err, stdout, stderr) {
exec('unzip -q -o -d "' + dest + '" "' + zipfile + '"', function (err, stdout, stderr) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unzip is not a valid command on Windows. You should use the unzip function in node-appc. To do this, first require node-appc:

var appc = require('node-appc');

Then unzip!

appc.zip.unzip(zipfile, dest, next);

@ypbnv
Copy link
Contributor Author

ypbnv commented Jul 6, 2017

@cb1kenobi Updated.

Copy link
Contributor

@cb1kenobi cb1kenobi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work as advertised. APPROVED.

@cb1kenobi cb1kenobi merged commit da10975 into tidev:master Jul 6, 2017
jquick-axway added a commit to jquick-axway/titanium_mobile that referenced this pull request Jul 24, 2017
- Bug was introduced on Mac and Windows as of PR tidev#9198.
- Note that this works-around a bug in CLI appc.zip.unzip() function, which is to be fixed later.
@jquick-axway
Copy link
Contributor

@ypbnv, you should check if there is a Windows bug with the following command as well...
node scons.js cleanbuild

The above does a clean, build, package, and install all at once. I've noticed the JavaScript for this command contains a copy of the old unzip code that you might want to change over to use appc.zip.unzip().

@ypbnv
Copy link
Contributor Author

ypbnv commented Jul 25, 2017

@jquick-axway Checked it - 'cleanbuild' on Windows does have the same problem. Since this PR introduces an issue - should I apply the changes from it for 'cleanbuild'?

@jquick-axway
Copy link
Contributor

@ypbnv, I would say go for it.

And go ahead and implement the work-around I've implemented in my PR #9244. Chris has approved it over e-mail. I'm okay with closing my PR if you implement it in this one.

@ypbnv
Copy link
Contributor Author

ypbnv commented Jul 26, 2017

@jquick-axway Here is a combined PR: #9259

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants