diff --git a/v3/UNRELEASED_CHANGELOG.md b/v3/UNRELEASED_CHANGELOG.md index 09daa80fe..7ca7cc6a3 100644 --- a/v3/UNRELEASED_CHANGELOG.md +++ b/v3/UNRELEASED_CHANGELOG.md @@ -25,6 +25,8 @@ After processing, the content will be moved to the main changelog and this file - Added missing window name to request headers, so it can be logged with the window ID by @jasikpark in [#4687](https://github.com/wailsapp/wails/pull/4687) +- Fix issue where `common:update:build-assets` command overwrites instead of updating plist files by @rxliuli in [#4636](https://github.com/wailsapp/wails/pull/4636) + ## Deprecated diff --git a/v3/internal/commands/build-assets.go b/v3/internal/commands/build-assets.go index 33182237c..b1f928407 100644 --- a/v3/internal/commands/build-assets.go +++ b/v3/internal/commands/build-assets.go @@ -13,6 +13,7 @@ import ( "github.com/leaanthony/gosod" "gopkg.in/yaml.v3" + "howett.net/plist" ) //go:embed build_assets @@ -251,11 +252,27 @@ func UpdateBuildAssets(options *UpdateBuildAssetsOptions) error { return err } + // Backup existing plist files before extraction + backups, err := backupPlistFiles(options.Dir) + if err != nil { + return err + } + + // Extract new assets (overwrites existing files) err = gosod.New(tfs).Extract(options.Dir, config) if err != nil { return err } + // Merge backed-up content into newly extracted plists + err = mergeBackupPlists(backups) + if err != nil { + return err + } + + // Clean up backup files + cleanupBackups(backups) + if !options.Silent { println("Successfully updated build assets in " + options.Dir) } @@ -266,3 +283,103 @@ func UpdateBuildAssets(options *UpdateBuildAssetsOptions) error { func normaliseName(name string) string { return strings.ToLower(strings.ReplaceAll(name, " ", "-")) } + +// mergeMaps recursively merges src into dst. +// For nested maps, it merges recursively. For other types, src overwrites dst. +func mergeMaps(dst, src map[string]any) { + for key, srcValue := range src { + if dstValue, exists := dst[key]; exists { + // If both are maps, merge recursively + srcMap, srcIsMap := srcValue.(map[string]any) + dstMap, dstIsMap := dstValue.(map[string]any) + if srcIsMap && dstIsMap { + mergeMaps(dstMap, srcMap) + continue + } + } + // Otherwise, src overwrites dst + dst[key] = srcValue + } +} + +// plistBackup holds the original path and backup path for a plist file +type plistBackup struct { + originalPath string + backupPath string +} + +// backupPlistFiles finds all .plist files in dir and renames them to .plist.bak +func backupPlistFiles(dir string) ([]plistBackup, error) { + var backups []plistBackup + + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() || !strings.HasSuffix(path, ".plist") { + return nil + } + + backupPath := path + ".bak" + if err := os.Rename(path, backupPath); err != nil { + return fmt.Errorf("failed to backup plist %s: %w", path, err) + } + backups = append(backups, plistBackup{originalPath: path, backupPath: backupPath}) + return nil + }) + + return backups, err +} + +// mergeBackupPlists merges the backed-up plist content into the newly extracted plists +func mergeBackupPlists(backups []plistBackup) error { + for _, backup := range backups { + // Read the backup (original user content) + backupContent, err := os.ReadFile(backup.backupPath) + if err != nil { + return fmt.Errorf("failed to read backup %s: %w", backup.backupPath, err) + } + + var backupDict map[string]any + if _, err := plist.Unmarshal(backupContent, &backupDict); err != nil { + return fmt.Errorf("failed to parse backup plist %s: %w", backup.backupPath, err) + } + + // Read the newly extracted plist + newContent, err := os.ReadFile(backup.originalPath) + if err != nil { + // New file might not exist if template didn't generate one for this path + continue + } + + var newDict map[string]any + if _, err := plist.Unmarshal(newContent, &newDict); err != nil { + return fmt.Errorf("failed to parse new plist %s: %w", backup.originalPath, err) + } + + // Merge: start with backup (user's content), apply new values on top + mergeMaps(backupDict, newDict) + + // Write merged result + file, err := os.Create(backup.originalPath) + if err != nil { + return fmt.Errorf("failed to create merged plist %s: %w", backup.originalPath, err) + } + + encoder := plist.NewEncoder(file) + encoder.Indent("\t") + if err := encoder.Encode(backupDict); err != nil { + file.Close() + return fmt.Errorf("failed to encode merged plist %s: %w", backup.originalPath, err) + } + file.Close() + } + return nil +} + +// cleanupBackups removes the backup files after successful merge +func cleanupBackups(backups []plistBackup) { + for _, backup := range backups { + os.Remove(backup.backupPath) + } +} diff --git a/v3/internal/commands/build-assets_test.go b/v3/internal/commands/build-assets_test.go index 2b16cafd5..c100d6197 100644 --- a/v3/internal/commands/build-assets_test.go +++ b/v3/internal/commands/build-assets_test.go @@ -6,6 +6,7 @@ import ( "testing" "gopkg.in/yaml.v3" + "howett.net/plist" ) func TestGenerateBuildAssets(t *testing.T) { @@ -265,3 +266,251 @@ func TestUpdateBuildAssets(t *testing.T) { }) } } + +func TestPlistMerge(t *testing.T) { + tempDir, err := os.MkdirTemp("", "wails-plist-test-*") + if err != nil { + t.Fatalf("Failed to create temp directory: %v", err) + } + defer os.RemoveAll(tempDir) + + buildDir := filepath.Join(tempDir, "build", "darwin") + err = os.MkdirAll(buildDir, 0755) + if err != nil { + t.Fatalf("Failed to create build directory: %v", err) + } + + existingPlistPath := filepath.Join(buildDir, "Info.plist") + existingPlist := ` + + + + CFBundleName + OldAppName + CFBundleVersion + 1.0.0 + NSCameraUsageDescription + This app needs camera access + NSMicrophoneUsageDescription + This app needs microphone access + +` + + err = os.WriteFile(existingPlistPath, []byte(existingPlist), 0644) + if err != nil { + t.Fatalf("Failed to write existing plist: %v", err) + } + + options := &UpdateBuildAssetsOptions{ + Dir: filepath.Join(tempDir, "build"), + Name: "TestApp", + ProductName: "NewAppName", + ProductVersion: "2.0.0", + ProductCompany: "Test Company", + ProductIdentifier: "com.test.app", + ProductDescription: "Test Description", + ProductCopyright: "© 2024 Test Company", + ProductComments: "Test Comments", + Silent: true, + } + + err = UpdateBuildAssets(options) + if err != nil { + t.Fatalf("UpdateBuildAssets failed: %v", err) + } + + mergedContent, err := os.ReadFile(existingPlistPath) + if err != nil { + t.Fatalf("Failed to read merged plist: %v", err) + } + + var mergedDict map[string]any + _, err = plist.Unmarshal(mergedContent, &mergedDict) + if err != nil { + t.Fatalf("Failed to parse merged plist: %v", err) + } + + if mergedDict["CFBundleName"] != "NewAppName" { + t.Errorf("Expected CFBundleName to be updated to 'NewAppName', got %v", mergedDict["CFBundleName"]) + } + + if mergedDict["CFBundleVersion"] != "2.0.0" { + t.Errorf("Expected CFBundleVersion to be updated to '2.0.0', got %v", mergedDict["CFBundleVersion"]) + } + + if mergedDict["NSCameraUsageDescription"] != "This app needs camera access" { + t.Errorf("Expected NSCameraUsageDescription to be preserved, got %v", mergedDict["NSCameraUsageDescription"]) + } + + if mergedDict["NSMicrophoneUsageDescription"] != "This app needs microphone access" { + t.Errorf("Expected NSMicrophoneUsageDescription to be preserved, got %v", mergedDict["NSMicrophoneUsageDescription"]) + } + + if mergedDict["CFBundleIdentifier"] != "com.test.app" { + t.Errorf("Expected CFBundleIdentifier to be 'com.test.app', got %v", mergedDict["CFBundleIdentifier"]) + } +} + +func TestNestedPlistMerge(t *testing.T) { + tests := []struct { + name string + existing map[string]any + new map[string]any + expected map[string]any + }{ + { + name: "simple overwrite", + existing: map[string]any{ + "key1": "oldValue", + }, + new: map[string]any{ + "key1": "newValue", + }, + expected: map[string]any{ + "key1": "newValue", + }, + }, + { + name: "preserve existing keys", + existing: map[string]any{ + "key1": "value1", + "key2": "value2", + }, + new: map[string]any{ + "key1": "newValue1", + }, + expected: map[string]any{ + "key1": "newValue1", + "key2": "value2", + }, + }, + { + name: "nested dict merge", + existing: map[string]any{ + "CustomConfig": map[string]any{ + "Setting1": "existingValue1", + "Setting2": "existingValue2", + }, + }, + new: map[string]any{ + "CustomConfig": map[string]any{ + "Setting1": "newValue1", + "Setting3": "newValue3", + }, + }, + expected: map[string]any{ + "CustomConfig": map[string]any{ + "Setting1": "newValue1", + "Setting2": "existingValue2", + "Setting3": "newValue3", + }, + }, + }, + { + name: "deeply nested merge", + existing: map[string]any{ + "Level1": map[string]any{ + "Level2": map[string]any{ + "deepKey1": "deepValue1", + "deepKey2": "deepValue2", + }, + }, + }, + new: map[string]any{ + "Level1": map[string]any{ + "Level2": map[string]any{ + "deepKey1": "newDeepValue1", + "deepKey3": "newDeepValue3", + }, + }, + }, + expected: map[string]any{ + "Level1": map[string]any{ + "Level2": map[string]any{ + "deepKey1": "newDeepValue1", + "deepKey2": "deepValue2", + "deepKey3": "newDeepValue3", + }, + }, + }, + }, + { + name: "mixed types - new dict replaces non-dict", + existing: map[string]any{ + "key1": "stringValue", + }, + new: map[string]any{ + "key1": map[string]any{ + "nested": "value", + }, + }, + expected: map[string]any{ + "key1": map[string]any{ + "nested": "value", + }, + }, + }, + { + name: "mixed types - new non-dict replaces dict", + existing: map[string]any{ + "key1": map[string]any{ + "nested": "value", + }, + }, + new: map[string]any{ + "key1": "stringValue", + }, + expected: map[string]any{ + "key1": "stringValue", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Make a copy of existing to avoid mutation issues + dst := deepCopyMap(tt.existing) + mergeMaps(dst, tt.new) + + if !mapsEqual(dst, tt.expected) { + t.Errorf("mergeMaps() got %v, expected %v", dst, tt.expected) + } + }) + } +} + +func deepCopyMap(m map[string]any) map[string]any { + result := make(map[string]any) + for k, v := range m { + if nested, ok := v.(map[string]any); ok { + result[k] = deepCopyMap(nested) + } else { + result[k] = v + } + } + return result +} + +func mapsEqual(a, b map[string]any) bool { + if len(a) != len(b) { + return false + } + for k, av := range a { + bv, ok := b[k] + if !ok { + return false + } + aMap, aIsMap := av.(map[string]any) + bMap, bIsMap := bv.(map[string]any) + if aIsMap && bIsMap { + if !mapsEqual(aMap, bMap) { + return false + } + } else if aIsMap != bIsMap { + return false + } else if av != bv { + return false + } + } + return true +}