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

[gym] iOS Export Options "Destination" Handling #16609

Merged
merged 3 commits into from Jul 29, 2020
Merged

[gym] iOS Export Options "Destination" Handling #16609

merged 3 commits into from Jul 29, 2020

Conversation

DomenicBianchi01
Copy link
Contributor

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.

Motivation and Context

gym supports passing in archive export options through the export_options parameter. When gym runs fastlane/gym/lib/assets/wrap_xcodebuild/xcbuild-safe.sh -exportArchive ..., it expects that a .ipa file (amongst other files) will be created afterwards.

However, if within export_options you specify destination: "upload", no external files are generated (including the .ipa) since destination: "upload tells xcodebuild to upload to iTunes Connect immediately.

As a result, if you specify destination: "upload", the archive will successfully upload to iTunes Connect (assuming you have configured everything else correctly), but you will get the following error:

ERROR [2020-06-15 09:29:18.37]: The generated archive is invalid, this can have various reasons:
ERROR [2020-06-15 09:29:18.37]: Usually it's caused by the `Skip Install` option in Xcode, set it to `NO`
ERROR [2020-06-15 09:29:18.37]: For more information visit https://developer.apple.com/library/ios/technotes/tn2215/_index.html
ERROR [2020-06-15 09:29:18.37]: Also, make sure to have a valid code signing identity and provisioning profile installed
ERROR [2020-06-15 09:29:18.37]: Follow this guide to setup code signing https://docs.fastlane.tools/codesigning/GettingStarted/
ERROR [2020-06-15 09:29:18.37]: If your intention was only to export an ipa be sure to provide a valid archive at the archive path.
ERROR [2020-06-15 09:29:18.37]: This error might also happen if your workspace/project file is not in the root directory of your project.
ERROR [2020-06-15 09:29:18.37]: To workaround that issue, you can wrap your calls to gym with
ERROR [2020-06-15 09:29:18.37]: `Dir.chdir('../path/to/dir/containing/proj') do`
ERROR [2020-06-15 09:29:18.37]: For an example you can check out
ERROR [2020-06-15 09:29:18.37]: https://github.com/artsy/emission-nebula/commit/44fe51a7fea8f7d52f0f77d6c3084827fe5dd59e

The purpose of this PR is to prevent Fastlane from displaying this misleading error.

Description

The changes in this PR are fairly straight forward.

I simply check if the export options plist contains the destination key with a value of upload. If it does, the code no longer executes the "move" related functions (since there are no files to move in the first place).

Testing Steps

Run gym using the following export options (depending on how your specific Xcode project is configured, you might need to include other options):

{
    "destination": "upload",
    "method": "app-store",
    "signingStyle": "automatic"
}

Also, specify export_team_id (an existing gym parameter)

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@DomenicBianchi01
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@DomenicBianchi01
Copy link
Contributor Author

@googlebot I fixed it.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@DomenicBianchi01 DomenicBianchi01 changed the title Gym: iOS Export Options "Destination" Handling [gym] iOS Export Options "Destination" Handling Jun 15, 2020
@joshdholtz
Copy link
Member

@DomenicBianchi01 I learned so much from this! 🤯 Hopefully I learned the right thing though lol... So if I’m understanding this correctly, you use gym to upload to App Store Connect by setting destination as upload? I had absolutely no idea this was a thing!

This code change totally makes sense and looks good from a review standpoint. I’m going to try to get something setup locally to test this because I really want to see this working on my own machine too 😇

@joshdholtz
Copy link
Member

Oh my goodness I did it! I uploaded an IPA through gym. This is 🔥

@DomenicBianchi01
Copy link
Contributor Author

DomenicBianchi01 commented Jul 29, 2020

@DomenicBianchi01 I learned so much from this! 🤯 Hopefully I learned the right thing though lol... So if I’m understanding this correctly, you use gym to upload to App Store Connect by setting destination as upload? I had absolutely no idea this was a thing!

This code change totally makes sense and looks good from a review standpoint. I’m going to try to get something setup locally to test this because I really want to see this working on my own machine too 😇

Correct! At the company I work at, we have multiple targets, each which need to be signed using a different team id/provisioning profiles/certificates/etc.

By setting the destination field, Xcode is actually smart enough to figure out exactly which iTMSTransporter team id needs to be used (sorry I don't know the actual name for this identifier, but it is NOT the team id shown in within the Signing & Configurations menu in Xcode) for a specific export and is therefore able to upload to App Store Connect in one shot.

Without these changes, I need to manually provide the iTMSTransporter team id to upload_to_app_store. Long story short, because we manage many different targets with different team ids, having to provide this extra ID is not ideal since setting the destination field means I don't need to worry about the ID at all.

@googlebot
Copy link

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@joshdholtz
Copy link
Member

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@joshdholtz
Copy link
Member

@DomenicBianchi01 Thanks again for adding support for this! I pushed a commit on to this to also add this for mac as well. I will get this merged in tomorrow morning and out in the next release 💪

Copy link
Member

@joshdholtz joshdholtz left a comment

Choose a reason for hiding this comment

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

This is 🔥 Thanks so much for making this change! I’m going to be adding this to some of my projects 😊 Really appreciate the contribution!

@joshdholtz joshdholtz merged commit 5c76b24 into fastlane:master Jul 29, 2020
@fastlane-bot
Copy link

Hey @DomenicBianchi01 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

Copy link

@fastlane-bot fastlane-bot left a comment

Choose a reason for hiding this comment

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

Congratulations! 🎉 This was released as part of fastlane 2.154.0 🚀

@fastlane fastlane locked and limited conversation to collaborators Sep 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants