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

Workaround for correctly exporting IPA when generic archive Xcodebuild bug occurs #4066

Closed
wants to merge 13 commits into from
Closed

Workaround for correctly exporting IPA when generic archive Xcodebuild bug occurs #4066

wants to merge 13 commits into from

Conversation

jcampbell05
Copy link
Contributor

Workaround for CocoaPods/CocoaPods#4178

In certain project configurations Xcodebuild creates a generic archive with the iOS App with Apple Watch App and the Apple Watch app in the products folder.

This is a small tweak to remove any Watch IPAs in the root of the archive (i.e not inside of a iOS App) since in pretty much all cases a developer wouldn't want / need this.

Hopefully this will increase the chances that gym will pick the correct IPA file when this bug occurs.

@jcampbell05 jcampbell05 changed the title Workaround for generic archive xcode bug. Workaround for correctly exporting IPA when generic archive Xcode bug occurs Apr 5, 2016
@jcampbell05 jcampbell05 changed the title Workaround for correctly exporting IPA when generic archive Xcode bug occurs Workaround for correctly exporting IPA when generic archive Xcodebuild bug occurs Apr 5, 2016
@samrobbins
Copy link
Contributor

@jcampbell05 what is your sense of how wide spread of a problem this is? Are others seeing the issue too and just not reporting it? Should we put an opt-in mechanism for this? I just don't want to break a workflow for someone that needs it to work this way currently.

@@ -58,6 +60,7 @@ def ipa_path
ErrorHandler.handle_empty_archive unless path
end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove?


# Does this application have a WatchKit target
def is_watchkit_ipa?(plist_path)
`/usr/libexec/PlistBuddy -c 'Print DTSDKName' '#{plist_path}' 2>&1`.match(/^\s*watchos2\.\d+\s*$/) != nil
Copy link
Contributor

Choose a reason for hiding this comment

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

For getting the path into the command, do #{plist_path.shellescape} without the quotes 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mfurtak I'll update this but we should think about having utility commands for these things in another PR, this is used elsewhere without shellescape unfortunately :(

@jcampbell05
Copy link
Contributor Author

@samrobbins This is a pretty wide-spread problem based on the issue filed with CocoaPods CocoaPods/CocoaPods#4178.

They may not have reported it since potentially it could be something wrong with their project triggering a bug with Xcode and CocoaPods (so not technically a Fastlane issue) but currently there isn't a straight forward reason to why this bug occurs (But I am working with CocoaPods to identify this, so either we or Apple can fix it).

That said, I am not against putting some sort of flag for now to switch this fix on or off depending on your needs if people need to archive the watch app itself.

The only reason we may not need this flag, is the only useful way to use a watch app archive is inside of a iOS App so potentially this use case is small and most developers would just archive the iOS App.

@samrobbins
Copy link
Contributor

@jcampbell05 I have reservations about the assumptions we are making with regard to identifying a project as iOS, tvOS etc. But after I asked the question I thought about it some more and perhaps having things work a certain way vs needing to know extra flags is better. @mfurtak thoughts on this last statement?

@mfurtak
Copy link
Contributor

mfurtak commented Apr 6, 2016

shellescape can be brought in with a require 'shellwords' 👍

@@ -72,6 +73,11 @@ def clear_old_files
end
end

def fix_archive
return unless Gym.config[:use_legacy_build_api]
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's do as @samrobbins suggests and put this behavior behind a feature switch for now.

Something like:

return unless Gym.config[:use_legacy_build_api] && !ENV['FASTLANE_GYM_USE_GENERIC_ARCHIVE_FIX'].nil?

Copy link
Member

Choose a reason for hiding this comment

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

This is confusing, with a lot of !, unless and nil?. Could we split it up to multiple lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

end
end


Choose a reason for hiding this comment

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

Extra blank line detected.

@jcampbell05
Copy link
Contributor Author

Updated and all the raised issues resolved.

@@ -103,6 +103,16 @@ def self.plain_options
UI.important "Using legacy build system - waiting for radar to be fixed: https://openradar.appspot.com/radar?id=4952000420642816"
end
end),
FastlaneCore::ConfigItem.new(key: :use_generic_archive_fix,

Choose a reason for hiding this comment

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

Align the elements of an array literal if they span more than one line.

@jcampbell05 jcampbell05 closed this Apr 8, 2016
@jcampbell05 jcampbell05 deleted the b/WorkaroundForGenericArchivesBug branch April 8, 2016 08:52
@@ -103,6 +103,16 @@ def self.plain_options
UI.important "Using legacy build system - waiting for radar to be fixed: https://openradar.appspot.com/radar?id=4952000420642816"
end
end),
FastlaneCore::ConfigItem.new(key: :use_generic_archive_fix,
env_name: "GYM_USE_GENERIC_ARCHIVE_FIX",
description: "Use fix for generic arhives https://github.com/CocoaPods/CocoaPods/issues/4178",
Copy link
Member

Choose a reason for hiding this comment

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

There is a spelling mistake arhives, could you submit a PR fixing it, or was it already fixed?

@fastlane fastlane locked and limited conversation to collaborators Feb 4, 2017
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

6 participants