Fix option conflicts in sigh

[mfurtak]

Adjust command option handling to stop using global_option for Commander options that are
generated for the default command (renew). Instead, they are now generated as normal options underneath that command.

The only major (negative?) change in behavior affects how help information is generated/shown by Commander.

Checklist

• I've run bundle exec rspec from the root directory to see all new and existing tests pass
• I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
• I've read the Contribution Guidelines
• I've updated the documentation if necessary.

Description

Current option handling in sigh

Current help output:

$ fastlane sigh -h
  sigh

  CLI for 'sigh' - Because you would rather spend your time building stuff than fighting provisioning

  Commands:
    download_all Downloads all valid provisioning profiles
    help         Display global or [command] help documentation
    manage       Manage installed provisioning profiles on your system.
    renew        Renews the certificate (in case it expired) and outputs the path to the generated file
    repair       Repairs all expired or invalid provisioning profiles
    resign       Resigns an existing ipa file with the given provisioning profile

  Global Options:
    --verbose
    --adhoc [VALUE]      Setting this flag will generate AdHoc profiles instead of App Store Profiles (SIGH_AD_HOC)
    --development [VALUE] Renew the development certificate instead of the production one (SIGH_DEVELOPMENT)
    --skip_install [VALUE] By default, the certificate will be added on your local machine. Setting this flag will skip this action (SIGH_SKIP_INSTALL)
    -f, --force [VALUE]  Renew provisioning profiles regardless of its state - to automatically add all devices for ad hoc profiles (SIGH_FORCE)
    -a, --app_identifier STRING The bundle identifier of your app (SIGH_APP_IDENTIFIER)
    -u, --username STRING Your Apple ID Username (SIGH_USERNAME)
    -b, --team_id STRING The ID of your Developer Portal team if you're in multiple teams (SIGH_TEAM_ID)
    -l, --team_name STRING The name of your Developer Portal team if you're in multiple teams (SIGH_TEAM_NAME)
    -n, --provisioning_name STRING The name of the profile that is used on the Apple Developer Portal (SIGH_PROVISIONING_PROFILE_NAME)
    --ignore_profiles_with_different_name [VALUE] Use in combination with :provisioning_name - when true only profiles matching this exact name will be downloaded
(SIGH_IGNORE_PROFILES_WITH_DIFFERENT_NAME)
    -o, --output_path STRING Directory in which the profile should be stored (SIGH_OUTPUT_PATH)
    -i, --cert_id STRING The ID of the code signing certificate to use (e.g. 78ADL6LVAA)  (SIGH_CERTIFICATE_ID)
    -c, --cert_owner_name STRING The certificate name to use for new profiles, or to renew with. (e.g. "Felix Krause") (SIGH_CERTIFICATE)
    -q, --filename STRING Filename to use for the generated provisioning profile (must include .mobileprovision) (SIGH_PROFILE_FILE_NAME)
    -w, --skip_fetch_profiles [VALUE] Skips the verification of existing profiles which is useful if you have thousands of profiles (SIGH_SKIP_FETCH_PROFILES)
    -z, --skip_certificate_verification [VALUE] Skips the verification of the certificates for every existing profiles. This will make sure the provisioning profile can be used
on the local machine (SIGH_SKIP_CERTIFICATE_VERIFICATION)
    -p, --platform [VALUE] Set the provisioning profile's platform (i.e. ios, tvos) (SIGH_PLATFORM)
    -h, --help           Display help documentation
    -v, --version        Display version information

  Author:
    Felix Krause <sigh@krausefx.com>

  Website:
    https://fastlane.tools

This should not work (but it does) since --force is not a valid option for the resign command

$ fastlane sigh resign --force
Path to ipa file: ^C

This is good and shows that --force is seen by the default command

$ fastlane sigh --force

+-------------------------------------+-------+
|           Summary for sigh 2.14.2           |
+-------------------------------------+-------+
| force                               | true  |
| adhoc                               | false |
| development                         | false |
| skip_install                        | false |
| ignore_profiles_with_different_name | false |
| skip_fetch_profiles                 | false |
| skip_certificate_verification       | false |
| platform                            | ios   |
+-------------------------------------+-------+

[11:37:26]: To not be asked about this value, you can specify it using 'username'
Your Apple ID Username: ^C

This should be setting the --provisioning_profile, but is incorrectly interpreted as the --platform from the default command

$ fastlane sigh resign -p abcd path.ipa

...

[1] pry(#<Sigh::CommandsGenerator>)> args
=> ["path.ipa"]
[2] pry(#<Sigh::CommandsGenerator>)> options
=> <Commander::Command::Options platform="abcd">

Revised option handling in sigh

Help for the top-level executable:

$ fastlane sigh -h
  sigh

  CLI for 'sigh' - Because you would rather spend your time building stuff than fighting provisioning

  Commands:
    download_all Downloads all valid provisioning profiles
    help         Display global or [command] help documentation
    manage       Manage installed provisioning profiles on your system.
    renew        Renews the certificate (in case it expired) and outputs the path to the generated file
    repair       Repairs all expired or invalid provisioning profiles
    resign       Resigns an existing ipa file with the given provisioning profile

  Global Options:
    --verbose             
    -h, --help           Display help documentation
    -v, --version        Display version information

  Author:
    Felix Krause <sigh@krausefx.com>

  Website:
    https://fastlane.tools

  GitHub:
    https://github.com/fastlane/sigh

Help for the renew (default) command:

$ fastlane sigh renew -h

  renew

  Usage: fastlane sigh renew

  Renews the certificate (in case it expired) and outputs the path to the generated file

  Options:
    --adhoc [VALUE]      Setting this flag will generate AdHoc profiles instead of App Store Profiles (SIGH_AD_HOC)
    --development [VALUE] Renew the development certificate instead of the production one (SIGH_DEVELOPMENT)
    --skip_install [VALUE] By default, the certificate will be added on your local machine. Setting this flag will skip this action (SIGH_SKIP_INSTALL)
    -f, --force [VALUE]  Renew provisioning profiles regardless of its state - to automatically add all devices for ad hoc profiles (SIGH_FORCE)
    -a, --app_identifier STRING The bundle identifier of your app (SIGH_APP_IDENTIFIER)
    -u, --username STRING Your Apple ID Username (SIGH_USERNAME)
    -b, --team_id STRING The ID of your Developer Portal team if you're in multiple teams (SIGH_TEAM_ID)
    -l, --team_name STRING The name of your Developer Portal team if you're in multiple teams (SIGH_TEAM_NAME)
    -n, --provisioning_name STRING The name of the profile that is used on the Apple Developer Portal (SIGH_PROVISIONING_PROFILE_NAME)
    --ignore_profiles_with_different_name [VALUE] Use in combination with :provisioning_name - when true only profiles matching this exact name will be downloaded
(SIGH_IGNORE_PROFILES_WITH_DIFFERENT_NAME)
    -o, --output_path STRING Directory in which the profile should be stored (SIGH_OUTPUT_PATH)
    -i, --cert_id STRING The ID of the code signing certificate to use (e.g. 78ADL6LVAA)  (SIGH_CERTIFICATE_ID)
    -c, --cert_owner_name STRING The certificate name to use for new profiles, or to renew with. (e.g. "Felix Krause") (SIGH_CERTIFICATE)
    -q, --filename STRING Filename to use for the generated provisioning profile (must include .mobileprovision) (SIGH_PROFILE_FILE_NAME)
    -w, --skip_fetch_profiles [VALUE] Skips the verification of existing profiles which is useful if you have thousands of profiles (SIGH_SKIP_FETCH_PROFILES)
    -z, --skip_certificate_verification [VALUE] Skips the verification of the certificates for every existing profiles. This will make sure the provisioning profile can be used
on the local machine (SIGH_SKIP_CERTIFICATE_VERIFICATION)
    -p, --platform [VALUE] Set the provisioning profile's platform (i.e. ios, tvos) (SIGH_PLATFORM)

The resign command now correctly throws an error when given the --force option

$ fastlane sigh resign --force
invalid option: --force

The default command still recognizes the --force flag

$ fastlane sigh --force

+-------------------------------------+-------+
|           Summary for sigh 2.14.2           |
+-------------------------------------+-------+
| force                               | true  |
| adhoc                               | false |
| development                         | false |
| skip_install                        | false |
| ignore_profiles_with_different_name | false |
| skip_fetch_profiles                 | false |
| skip_certificate_verification       | false |
| platform                            | ios   |
+-------------------------------------+-------+

[11:36:19]: To not be asked about this value, you can specify it using 'username'
Your Apple ID Username: ^C

The resign command correctly receives the --provisioning_profile option

$ fastlane sigh resign -p abcd path.ipa

...

[1] pry(#<Sigh::CommandsGenerator>)> args
=> ["path.ipa"]
[2] pry(#<Sigh::CommandsGenerator>)> options
=> <Commander::Command::Options provisioning_profile=[["abcd"]]>

Motivation and Context

#6169 introduced a new option for sigh. Unfortunately, it accidentally conflicts with the -p short flag as used by the sigh resign command. This is one example, but there are other sigh options in use that also have short flag conflicts, such as -i. These problems were captured in #7935 and #8076

This is a problem for all fastlane tools, not just sigh, but I'm testing out this approach here to see how it works, and gather feedback.

[mfurtak]

@mgrebenets @hjanuschka @KrauseFx It would be great to have some help testing sigh with these changes to try to identify any regressions.

[mfurtak]

The CI failure does not appear to be related to my code, and all tests pass when I run them locally 😢

[mfurtak]

Rebased and pushed to pick up the test fix

[mgrebenets]

@mfurtak Will give it a test as soon as I can.

I have resigner CI script at work, which uses both -i and -p options. Just 2 weeks ago I upped resigner to use fastlane 2.12.0 and had to switch to long options to get it working.

[mgrebenets]

@mfurtak

So I've tested with the following command:

bundle exec fastlane sigh resign -i "iPhone Distribution: Company" \
  -p au.com.original=au.com.new.mobileprovision \
  -p au.com.original.watchkitapp=au.com.new.watchkitapp.mobileprovision \
  -p au.com.original.watchkitextension=au.com.new.watchkitextension.mobileprovision \
  -p au.com.original.apnscrypto=au.com.new.apnscrypto.mobileprovision \
  -p au.com.original.irdisplay=au.com.new.irdisplay.mobileprovision \
  --verbose \
  App.ipa

And it unfolded to the following call to resign.sh:

/Users/grebenma/.rvm/gems/ruby-2.2.2/bundler/gems/fastlane-3a11a5d8f976/sigh/lib/assets/resign.sh \
  App.ipa 572F2AC963CC60EF3C0A5920C1E781B86EBA57CC \
  -p au.com.original=/Users/grebenma/Projects/tmp/sigh-issue/au.com.new.mobileprovision \
  -p au.com.original.watchkitapp=/Users/grebenma/Projects/tmp/sigh-issue/au.com.new.watchkitapp.mobileprovision \
  -p au.com.original.watchkitextension=/Users/grebenma/Projects/tmp/sigh-issue/au.com.new.watchkitextension.mobileprovision \
  -p au.com.original.apnscrypto=/Users/grebenma/Projects/tmp/sigh-issue/au.com.new.apnscrypto.mobileprovision \
  -p au.com.original.irdisplay=/Users/grebenma/Projects/tmp/sigh-issue/au.com.new.irdisplay.mobileprovision \
  -v \
  App.ipa

Which is correct and is exactly what I am expecting to see.

If I run the same command using latest code from master, I get the "Signing Identity:" prompt, because the -i option was shadowed.

If I change -i to --signing_identity I will get the following command on master:

/Users/grebenma/Projects/oss/fastlane/sigh/lib/assets/resign.sh \
   App.ipa \
  SIGNINGIDHASH \
  -p /Users/grebenma/Projects/tmp/sigh-issue/au.com.old.apnscrypto.mobileprovision \
  -v \
  APP.ipa

Here the -p is shadowed.

---

So as a summary, I can confirm that this change fixes issues I had and short options work properly for resign subcommand.

[mgrebenets]

Tested with our resign scripts, looks good 👍

[hjanuschka]

🎉

[mgrebenets]

Could this be tested with a spec?

[hjanuschka]

#8076 (comment) should be a base for a spec.

[mfurtak]

Thanks for doing some manual verification. I'll see if I can get a regression test in place. 👍

[nafu]

The only major (negative?) change in behavior affects how help information is generated/shown by Commander.

Looks good to me 👍 If this behaivior will be also applied to other tools.

[mfurtak]

OK, I've added some tests that will catch a regression, so I'll merge this now 👍