Migrate packages #287

godrei · 2025-09-12T11:12:45Z

No description provided.

…l packages

bitrise-ip-bot · 2025-09-12T11:13:22Z

Summary

This PR migrates the codebase from v1 to v2 module structure, updating import paths and API calls. The migration includes moving packages like certificateutil, profileutil, export, exportoptions, and plistutil to the v2 module, replacing third-party dependencies with internal ones, and modernizing APIs with new patterns like ProfileProvider.

Walkthrough

File	Summary
go.mod, go.sum	Updated dependencies: moved packages to direct deps, removed pkg/errors, updated go-utils/v2
certificateutil/*.go	Added new certificateutil package with critical compilation error: 'pettern' typo should be 'pattern'
profileutil/*.go	Added new profileutil package with variable naming bugs in GetName() function
export/*.go	Added new export package with missing nil checks that could cause panics
exportoptions/*.go	Added new exportoptions package with String() methods returning nil instead of error
plistutil/plistutil.go	Added new plistutil package with potential type casting issues for numeric values
autocodesign/certdownloader/*.go	Incorrect import path update will cause compilation error
devportalservice/FileManager.go	Replaced custom mock with mockery-generated mock, but has fs.FileMode vs os.FileMode mismatch
autocodesign/localcodesignasset/*.go	Updated to v2 profileutil APIs with missing error handling and performance issues
artifacts/.go, autocodesign/.go, codesign/.go, exportoptionsgenerator/.go, metaparser/.go, xcarchive/.go, _integration_tests/*.go	Successfully migrated import paths from v1 to v2 and updated API calls

bitrise-ip-bot

This is an AI-generated review. Please review it carefully.

Actionable comments posted: 5

bitrise-ip-bot · 2025-09-12T11:17:09Z

certificateutil/info_model.go

+	for _, certificate := range certificates {
+		if certificate != nil {
+			infos = append(infos, NewCertificateInfo(*certificate, nil))
+		}
+	}


🐛 Bug

Potential nil pointer dereference: The code checks if certificate != nil but then immediately dereferences it with *certificate. If the certificate pointer becomes nil between the check and dereference due to concurrent access, this could cause a panic.

🔄 Suggestion:

Suggested change

for _, certificate := range certificates {

if certificate != nil {

infos = append(infos, NewCertificateInfo(*certificate, nil))

}

}

for _, certificate := range certificates {

if certificate != nil {

cert := *certificate

infos = append(infos, NewCertificateInfo(cert, nil))

}

}

bitrise-ip-bot · 2025-09-12T11:17:09Z

certificateutil/info_model.go

+	for _, certificate := range certificates {
+		if certificate != nil {
+			infos = append(infos, NewCertificateInfo(*certificate, nil))
+		}
+	}


🐛 Bug

Potential nil pointer dereference: The code checks if certificate != nil but then immediately dereferences it with *certificate. If the certificate pointer becomes nil between the check and dereference due to concurrent access, this could cause a panic.

🔄 Suggestion:

Suggested change

for _, certificate := range certificates {

if certificate != nil {

infos = append(infos, NewCertificateInfo(*certificate, nil))

}

}

for _, certificate := range certificates {

if certificate != nil {

cert := *certificate

infos = append(infos, NewCertificateInfo(cert, nil))

}

}

bitrise-ip-bot · 2025-09-12T11:17:10Z

certificateutil/util.go

+	if len(certificates) != len(privateKeys) {
+		return nil, errors.New("pkcs12: different number of certificates and private keys found")
+	}


🐛 Bug

Inconsistent error handling: The function checks the length of certificates and privateKeys slices without first validating that both slices are non-nil, which could cause a panic if either slice is nil.

🔄 Suggestion:

Suggested change

if len(certificates) != len(privateKeys) {

return nil, errors.New("pkcs12: different number of certificates and private keys found")

}

if certificates == nil || privateKeys == nil {

return nil, errors.New("pkcs12: certificates or private keys slice is nil")

}

if len(certificates) != len(privateKeys) {

return nil, errors.New("pkcs12: different number of certificates and private keys found")

}

bitrise-ip-bot · 2025-09-12T11:17:10Z

certificateutil/util.go

+	pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`
+	matches := regexp.MustCompile(pattern).FindAllString(out, -1)


🐛 Bug

Regex compilation inefficiency: The regex pattern is compiled inside the function on every call. For better performance and to avoid potential regex compilation errors at runtime, this should be compiled once as a package-level variable.

🔄 Suggestion:

Suggested change

pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`

matches := regexp.MustCompile(pattern).FindAllString(out, -1)

var certificatePattern = regexp.MustCompile(`(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`)

func normalizeFindCertificateOut(out string) ([]string, error) {

certificateContents := []string{}

matches := certificatePattern.FindAllString(out, -1)

bitrise-ip-bot · 2025-09-12T11:17:10Z

export/filter.go

+	missingEntitlements := []string{}
+
+	for key := range targetEntitlements {
+		_, known := profileutil.KnownProfileCapabilitiesMap[profileType][key]


🐛 Bug

Potential nil pointer dereference: The code accesses profileutil.KnownProfileCapabilitiesMap[profileType][key] without checking if profileutil.KnownProfileCapabilitiesMap[profileType] exists first. This will cause a panic if an unsupported profileType (like ProfileTypeTvOs) is passed.

🔄 Suggestion:

Suggested change

_, known := profileutil.KnownProfileCapabilitiesMap[profileType][key]

profileCapabilities, exists := profileutil.KnownProfileCapabilitiesMap[profileType]

if !exists {

continue

}

_, known := profileCapabilities[key]

bitrise-ip-bot

This is an AI-generated review. Please review it carefully.

Actionable comments posted: 3

bitrise-ip-bot · 2025-09-12T11:17:55Z

certificateutil/util.go

+}
+
+func installedCodesigningCertificateNamesFromOutput(out string) ([]string, error) {
+	pettern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"`


🐛 Bug

Variable name typo: 'pettern' should be 'pattern'. This will cause a compilation error as the variable is used on the next line but never declared correctly.

🔄 Suggestion:

Suggested change

pettern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"`

pattern := `^[0-9]+\) (?P<hash>.*) "(?P<name>.*)"`

bitrise-ip-bot · 2025-09-12T11:17:55Z

export/filter.go

+	missingEntitlements := []string{}
+
+	for key := range targetEntitlements {
+		_, known := profileutil.KnownProfileCapabilitiesMap[profileType][key]


🐛 Bug

Potential map access panic when profileType doesn't exist as a key in KnownProfileCapabilitiesMap. If an unsupported profile type (like ProfileTypeTvOs) is passed, this will cause a runtime panic when trying to access the nested map.

🔄 Suggestion:

Suggested change

_, known := profileutil.KnownProfileCapabilitiesMap[profileType][key]

capabilitiesMap, exists := profileutil.KnownProfileCapabilitiesMap[profileType]

if !exists {

continue

}

_, known := capabilitiesMap[key]

bitrise-ip-bot · 2025-09-12T11:17:55Z

profileutil/util.go

+		return nil, err
+	}
+
+	pattern := filepath.Join(pathutil.EscapeGlobPath(absProvProfileDirPath), "*"+ext)


🐛 Bug

Function pathutil.EscapeGlobPath does not exist in the github.com/bitrise-io/go-utils/pathutil package. This will cause a compilation error when the code is built.

🔄 Suggestion:

Suggested change

pattern := filepath.Join(pathutil.EscapeGlobPath(absProvProfileDirPath), "*"+ext)

pattern := filepath.Join(absProvProfileDirPath, "*"+ext)

bitrise-ip-bot

This is an AI-generated review. Please review it carefully.

Actionable comments posted: 4

bitrise-ip-bot · 2025-09-26T12:12:46Z

autocodesign/localcodesignasset/profileconverter.go

+			return nil, err
+		}
+	}
 	content, err := os.ReadFile(pth)


🐛 Bug

Potential nil pointer dereference when pth is empty string. If both iOS and tvOS profile lookups fail to find a profile (both return empty path), pth will be empty and os.ReadFile will fail with a misleading error.

🔄 Suggestion:

Suggested change

content, err := os.ReadFile(pth)

if pth == "" {

return nil, fmt.Errorf("provisioning profile with UUID %s not found", info.UUID)

}

content, err := os.ReadFile(pth)

bitrise-ip-bot · 2025-09-26T12:12:47Z

plistutil/plistutil.go

+	casted, ok := value.(uint64)
+	if !ok {
+		return 0, false
+	}


🐛 Bug

Potential type casting issue: plist library may unmarshal integers as different numeric types (int, int64, float64) depending on the source format and value range, but this method only accepts uint64.

🔄 Suggestion:

Suggested change

casted, ok := value.(uint64)

if !ok {

return 0, false

}

// Try direct uint64 cast first

if casted, ok := value.(uint64); ok {

return casted, true

}

// Handle other numeric types that plist might unmarshal to

switch v := value.(type) {

case int:

if v >= 0 {

return uint64(v), true

}

case int64:

if v >= 0 {

return uint64(v), true

}

case float64:

if v >= 0 && v == float64(uint64(v)) {

return uint64(v), true

}

}

return 0, false

bitrise-ip-bot · 2025-09-26T12:12:47Z

plistutil/plistutil.go

+		casted, ok := v.(uint64)
+		if !ok {
+			return nil, false
+		}


🐛 Bug

Potential type casting issue: similar to GetUInt64, this array method may fail when plist library unmarshals integers as different numeric types (int, int64, float64).

🔄 Suggestion:

Suggested change

casted, ok := v.(uint64)

if !ok {

return nil, false

}

// Try direct uint64 cast first

if casted, ok := v.(uint64); ok {

array = append(array, casted)

continue

}

// Handle other numeric types that plist might unmarshal to

switch val := v.(type) {

case int:

if val >= 0 {

array = append(array, uint64(val))

continue

}

case int64:

if val >= 0 {

array = append(array, uint64(val))

continue

}

case float64:

if val >= 0 && val == float64(uint64(val)) {

array = append(array, uint64(val))

continue

}

}

return nil, false

bitrise-ip-bot · 2025-09-26T12:12:47Z

profileutil/plist_data.go

+func (profile PlistData) GetName() string {
+	data := plistutil.PlistData(profile)
+	uuid, _ := data.GetString("Name")
+	return uuid


🐛 Bug

Returning wrong variable: should return 'name' instead of 'uuid'

🔄 Suggestion:

Suggested change

return uuid

return name

godrei added 7 commits August 15, 2025 13:52

Move certificateutil, profileutil, export, exportoptions and plistuti…

dc07914

…l packages

Replace github.com/pkg/errors with the stdlib errors package

0971cb7

Replace howett.net/plist with github.com/bitrise-io/go-plist

5e39744

Migrate v1 utility libs

8c39300

Migrate v1 utility libs and fix tests

29e3a59

Make exportoptions.WritePlistToFile private

48e384c

Cleanup export package

ea734cd

Merge branch 'master' into migrate-packages

680219d

bitrise-ip-bot reviewed Sep 12, 2025

View reviewed changes

Migrate profileutil

6e55534

bitrise-ip-bot reviewed Sep 26, 2025

View reviewed changes

		pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`
		matches := regexp.MustCompile(pattern).FindAllString(out, -1)

-	pattern := `(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`
-	matches := regexp.MustCompile(pattern).FindAllString(out, -1)
+var certificatePattern = regexp.MustCompile(`(?s)(-----BEGIN CERTIFICATE-----.*?-----END CERTIFICATE-----)`)
+func normalizeFindCertificateOut(out string) ([]string, error) {
+	certificateContents := []string{}
+	matches := certificatePattern.FindAllString(out, -1)

-		_, known := profileutil.KnownProfileCapabilitiesMap[profileType][key]
+		profileCapabilities, exists := profileutil.KnownProfileCapabilitiesMap[profileType]
+		if !exists {
+			continue
+		}
+		_, known := profileCapabilities[key]

	pettern := `^[0-9]+\) (?P<hash>.) "(?P<name>.)"`
	pattern := `^[0-9]+\) (?P<hash>.) "(?P<name>.)"`

	pattern := filepath.Join(pathutil.EscapeGlobPath(absProvProfileDirPath), "*"+ext)
	pattern := filepath.Join(absProvProfileDirPath, "*"+ext)

-	casted, ok := value.(uint64)
-	if !ok {
-		return 0, false
-	}
+	// Try direct uint64 cast first
+	if casted, ok := value.(uint64); ok {
+		return casted, true
+	}
+	// Handle other numeric types that plist might unmarshal to
+	switch v := value.(type) {
+	case int:
+		if v >= 0 {
+			return uint64(v), true
+		}
+	case int64:
+		if v >= 0 {
+			return uint64(v), true
+		}
+	case float64:
+		if v >= 0 && v == float64(uint64(v)) {
+			return uint64(v), true
+		}
+	}
+	return 0, false

Migrate packages #287

Are you sure you want to change the base?

Migrate packages #287

Uh oh!

Conversation

godrei commented Sep 12, 2025

Uh oh!

bitrise-ip-bot commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Walkthrough

Uh oh!

bitrise-ip-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitrise-ip-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bitrise-ip-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bitrise-ip-bot commented Sep 12, 2025 •

edited

Loading