Skip to content

MacOS archive fix #73

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 17 commits into from
Closed

MacOS archive fix #73

wants to merge 17 commits into from

Conversation

@BirmacherAkos
Copy link
Contributor

@BirmacherAkos BirmacherAkos commented Apr 12, 2018

No description provided.

godrei
godrei reviewed Apr 13, 2018
View reviewed changes
cmd/common.go Outdated
func collectIpaExportCodeSignGroups(tool Tool, archive xcarchive.IosArchive, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel) ([]export.IosCodeSignGroup, error) {
iosCodeSignGroups := []export.IosCodeSignGroup{}
func collectIpaExportCodeSignGroups(tool Tool, archive Archive, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel) ([]interface{}, error) {
iosCodeSignGroups := []interface{}{}
Copy link
Contributor

@godrei godrei Apr 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

this is not the best.
i think we should introduce a new interface for the codesigngroups.

I would modify the ios and macos codesigngroups.

Both have the same properties:

Certificate        certificateutil.CertificateInfoModel
BundleIDProfileMap map[string]profileutil.ProvisioningProfileInfoModel

make them unexported and create a function to access them.

macos has an extra field:

InstallerCertificate *certificateutil.CertificateInfoModel

i would recommend to also make this unexported and create an accessor function for this and also create it for ios and always return nil. This way we can define a common interface for them:

Also if we add IsMacos function to the structs and to the interface it would be cast them later.

Codesigngroup {
	Certificate() certificateutil.CertificateInfoModel
	BundleIDProfileMap() map[string]profileutil.ProvisioningProfileInfoModel
	InstallerCertificate() *certificateutil.CertificateInfoModel
}

bitrise-devs-bot
bitrise-devs-bot reviewed Apr 23, 2018
View reviewed changes
cmd/common.go Outdated
BundleIDProfileMap: selectedBundleIDProfileMap,
var collectedCodeSignGroup export.CodeSignGroup
if isMacArchive {
collectedCodeSignGroup = export.NewMacGroup(*selectedCertificate, nil, selectedBundleIDProfileMap)
Copy link

@bitrise-devs-bot bitrise-devs-bot Apr 23, 2018

Choose a reason for hiding this comment

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

we should handle the installerCertificate for mac as well

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/common.go Outdated
fmt.Println()
fmt.Println()
log.Infof("Codesign settings will be used for %s ipa export:", exportMethod(iosCodeSignGroup))
log.Infof("Codesign settings will be used for %s ipa export:", exportMethod(collectedCodeSignGroup))
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

in case of macos, we do not export and ipa file.

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/common.go
return nil
}

func getFilesToExport(archivePath string, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel, tool Tool) ([]certificateutil.CertificateInfoModel, []profileutil.ProvisioningProfileInfoModel, error) {
Copy link
Contributor

@godrei godrei Apr 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

should handle the macos installer certificate as well?

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/common.go Outdated
return certificatesToExport, profilesToExport, nil
}

func exportCertificatesOnly(tool Tool, certificate certificateutil.CertificateInfoModel, installedCertificates []certificateutil.CertificateInfoModel, certificatesToExport []certificateutil.CertificateInfoModel) ([]certificateutil.CertificateInfoModel, error) {
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

why is it moved to a separate function?

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/common.go Outdated
return certificatesToExport, profilesToExport, nil
}

func exportCertificatesAndProfiles(macOS bool, archive Archive, tool Tool, certificate certificateutil.CertificateInfoModel,
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

it just collects, but not exports these files

Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

macOS can be detected based on the archive, no need for this input.

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/common.go Outdated
return nil, nil, err
}

var ipaExportCodeSignGroups []export.CodeSignGroup
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

there is no ipa in case of macos, use just exportCodesignGroup for ex.

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/utils.go
)

func extractCertificatesAndProfiles(codeSignGroups ...export.IosCodeSignGroup) ([]certificateutil.CertificateInfoModel, []profileutil.ProvisioningProfileInfoModel) {
func extractCertificatesAndProfiles(codeSignGroups ...export.CodeSignGroup) ([]certificateutil.CertificateInfoModel, []profileutil.ProvisioningProfileInfoModel) {
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

should handle installer certs as well

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/utils.go
return certificatesByTeam
}

func getIOSCodeSignGroup(archivePath string, installedCertificates []certificateutil.CertificateInfoModel) (xcarchive.IosArchive, *export.IosCodeSignGroup, error) {
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

why is there separate function for getIOSCodeSignGroup and getMacOSCodeSignGroup?

godrei
godrei reviewed Apr 23, 2018
View reviewed changes
cmd/utils.go Outdated
return codeSignGroups
}

func analyzeArchive(signingIdentity string, bundleIDProfileInfoMap map[string]profileutil.ProvisioningProfileInfoModel, installedCertificates []certificateutil.CertificateInfoModel) (certificateutil.CertificateInfoModel, map[string]profileutil.ProvisioningProfileInfoModel, error) {
Copy link
Contributor

@godrei godrei Apr 23, 2018

Choose a reason for hiding this comment

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

do we need this function?

@godrei
Copy link
Contributor

godrei commented Apr 25, 2018

please remove appSelectionError

godrei
godrei reviewed Apr 25, 2018
View reviewed changes
cmd/common.go
func collectIpaExportCodeSignGroups(tool Tool, archive xcarchive.IosArchive, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel) ([]export.IosCodeSignGroup, error) {
iosCodeSignGroups := []export.IosCodeSignGroup{}
func collectIpaExportCodeSignGroups(tool Tool, archive Archive, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel) ([]export.CodeSignGroup, error) {
collectedCodeSignGroups := []export.CodeSignGroup{}
Copy link
Contributor

@godrei godrei Apr 25, 2018

Choose a reason for hiding this comment

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

var collectedCodeSignGroups []export.CodeSignGroup

godrei
godrei reviewed Apr 25, 2018
View reviewed changes
cmd/common.go
iosCodeSignGroups := []export.IosCodeSignGroup{}
func collectIpaExportCodeSignGroups(tool Tool, archive Archive, installedCertificates []certificateutil.CertificateInfoModel, installedProfiles []profileutil.ProvisioningProfileInfoModel) ([]export.CodeSignGroup, error) {
collectedCodeSignGroups := []export.CodeSignGroup{}
_, isMacArchive := archive.(xcarchive.MacosArchive)
Copy link
Contributor

@godrei godrei Apr 25, 2018

Choose a reason for hiding this comment

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

in case of macos export we have different possible export methods: https://github.com/bitrise-steplib/steps-xcode-archive-mac/blob/master/step.yml#L40

godrei
godrei reviewed Apr 25, 2018
View reviewed changes
cmd/common.go
BundleIDProfileMap: selectedBundleIDProfileMap,
var collectedCodeSignGroup export.CodeSignGroup
if isMacArchive {
installedInstallerCertificates := []certificateutil.CertificateInfoModel{}
Copy link
Contributor

@godrei godrei Apr 25, 2018

Choose a reason for hiding this comment

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

var installedInstallerCertificates []certificateutil.CertificateInfoModel

@BirmacherAkos
Copy link
Contributor Author

BirmacherAkos commented Jul 3, 2018

Merged in other PR: #83

@BirmacherAkos BirmacherAkos deleted the birmachera branch July 6, 2018 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@godrei godrei godrei left review comments

@bitrise-devs-bot bitrise-devs-bot bitrise-devs-bot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@BirmacherAkos @godrei @bitrise-devs-bot