Parse options correctly

[Mitko-Kerezov]

Fix 1
Options have already passed through nopt module in the build file. Nopt successfully parses all options which results in argv.remain being an empty array.
This ultimately leads to a bug in the parseOpts function where the options undergo a second parsing from nopt.
The end result is - this options are not respected at all.

Fix 2
parseOpts function referenced this.root but the this object is not the new Api as expected. This leads to an exception upon calling path.relative(this.root, ...).

[Mitko-Kerezov]

Ping @vladimir-kotikov

[Mitko-Kerezov]

Ping @infil00p for review

[vladimir-kotikov]

@Mitko-Kerezov, sorry for delay. The second fix LGTM.
Regarding the first - looks like it breaks the scenario when platform-specific options are passed behind -- arguments delimiter. I.e. with this patch applied the following command cordova build android -- --ant still uses gradle.

Also in order to proceed with this PR could you please submit an Apache CLA and file a JIRA for these bugs as described here: https://github.com/apache/cordova-android/blob/master/CONTRIBUTING.md

[nikhilkh]

This looks like a great fix to merge. @Mitko-Kerezov Did you get a chance to file the ICLA?

[Mitko-Kerezov]

@nikhilkh I've already filed the ICLA some time in 2015 and have already contributed to cordova. (Can't find the email though but I have merged pull requests)

@vladimir-kotikov regarding your comment, I don't believe cordova build android -- --ant should work at all. The -- delimiter stops the options parsing for nopt module and places the remaining args in argv.remain. This worked prior to the fix, however this is faulty behavior IMO.
Basically we'd either want cordova build android --ant or cordova build android -- --ant to work. The former is the right way IMO, because in the latter the usage of -- basically suggests "I want to stop passing options and start passing actual arguments - filenames, etc"

[vladimir-kotikov]

@Mitko-Kerezov, my bad, i missed you name in CLA list

Regarding the

the usage of -- basically suggests "I want to stop passing options and start passing actual arguments - filenames, etc"

I'd say it tells "do not consider the options behind me, just pass them to underlying script" (see, for example, istanbul docs and npm run help).

There is two possible ways to run build: platforms/android/cordova/build --ant and cordova build android -- --ant usage. In the first case -- delimiter is not needed, indeed, because you're running the script directly, but in the second it means that CLI will ignore these options and just pass them to android's build.

It is also still necessary for compatibility reasons (IMO, changing the way how CLI arguments are treated is a breaking change).

[Mitko-Kerezov]

@vladimir-kotikov
I've restructured the fixes and have separated them in two commits.
Regarding the options I've considered your words and have taken a different approach.
I didn't know that cordova build android -- --ant was meant to be used that way, however platforms/android/cordova/build --ant does not work for cordova-android@5.0.0.
It does work however for the previous cordova-android@4.1.1.
I've backtracked a bit in the commits before your major refactoring and have come across the previous implementation.
By passing args.slice(2) all options have been passed to the build script. This is why I've passed original instead of remain. Moreover nopt seems to parse all options and leaves nothing inside remain even if -- delimiter is passed which is somewhat strange but something to live with I guess.

[vladimir-kotikov]

Verified and it seems to work fine now with all possible combinations of -- and other args (i tested with --ant, --keystore and --alias )
Thanks, @Mitko-Kerezov

[nikhilkh]

Wohoo! Good to see this is getting committed!