Skip to content

Commit 8768561

Browse files
IhateTrainsIhateTrains
authored
Fix the I:R CoA extraction not working and causing Imperator crashes (#3107) #patch
1 parent e59c9d7 commit 8768561

4 files changed

Lines changed: 426 additions & 23 deletions

File tree

‎ImperatorToCK3.UnitTests/CommonUtils/FileHelperTests.cs‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,83 @@ public void OutputGuiContainer_handlesFileInPlaceOfGuiDirectory() {
7676
Assert.False(Directory.Exists(expectedGuiDir));
7777
Assert.True(File.Exists(collisionFile));
7878
}
79+
80+
[Fact]
81+
public void DeleteDirectoryWithRetries_deletesDirectoryWithReadOnlyContents() {
82+
var targetDir = Path.Combine(tempRoot, "readOnlyDir", "nested");
83+
Directory.CreateDirectory(targetDir);
84+
85+
var rootDir = Path.GetDirectoryName(targetDir)!;
86+
var filePath = Path.Combine(targetDir, "topbar.gui");
87+
File.WriteAllText(filePath, "foo");
88+
File.SetAttributes(filePath, FileAttributes.ReadOnly);
89+
File.SetAttributes(targetDir, FileAttributes.ReadOnly);
90+
File.SetAttributes(rootDir, FileAttributes.ReadOnly);
91+
92+
FileHelper.DeleteDirectoryWithRetries(rootDir);
93+
94+
Assert.False(Directory.Exists(rootDir));
95+
}
96+
97+
[Fact]
98+
public void EnsureFileIsWritable_noopsWhenFileDoesNotExist() {
99+
var missing = Path.Combine(tempRoot, "nonexistent.txt");
100+
101+
var exception = Record.Exception(() => FileHelper.EnsureFileIsWritable(missing));
102+
103+
Assert.Null(exception);
104+
}
105+
106+
[Fact]
107+
public void EnsureFileIsWritable_noopsWhenPathIsDirectory() {
108+
var dir = Path.Combine(tempRoot, "someDir");
109+
Directory.CreateDirectory(dir);
110+
111+
var exception = Record.Exception(() => FileHelper.EnsureFileIsWritable(dir));
112+
113+
Assert.Null(exception);
114+
}
115+
116+
[Fact]
117+
public void EnsureFileIsWritable_makesReadOnlyFileWritable() {
118+
var filePath = Path.Combine(tempRoot, "readonly.txt");
119+
File.WriteAllText(filePath, "content");
120+
SetReadOnly(filePath);
121+
Assert.False(IsWritable(filePath));
122+
123+
FileHelper.EnsureFileIsWritable(filePath);
124+
125+
Assert.True(IsWritable(filePath));
126+
// Must not corrupt content.
127+
Assert.Equal("content", File.ReadAllText(filePath));
128+
}
129+
130+
[Fact]
131+
public void EnsureFileIsWritable_isIdempotentOnAlreadyWritableFile() {
132+
var filePath = Path.Combine(tempRoot, "writable.txt");
133+
File.WriteAllText(filePath, "data");
134+
Assert.True(IsWritable(filePath));
135+
136+
var exception = Record.Exception(() => FileHelper.EnsureFileIsWritable(filePath));
137+
138+
Assert.Null(exception);
139+
Assert.True(IsWritable(filePath));
140+
}
141+
142+
private static void SetReadOnly(string filePath) {
143+
if (OperatingSystem.IsWindows()) {
144+
File.SetAttributes(filePath, FileAttributes.ReadOnly);
145+
} else {
146+
var mode = File.GetUnixFileMode(filePath);
147+
File.SetUnixFileMode(filePath, mode & ~UnixFileMode.UserWrite);
148+
}
149+
}
150+
151+
private static bool IsWritable(string filePath) {
152+
if (OperatingSystem.IsWindows()) {
153+
return !File.GetAttributes(filePath).HasFlag(FileAttributes.ReadOnly);
154+
}
155+
return File.GetUnixFileMode(filePath).HasFlag(UnixFileMode.UserWrite);
156+
}
79157
}
80158
}

‎ImperatorToCK3.UnitTests/Imperator/WorldTests.cs‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.IO;
33
using System.Reflection;
4+
using ImperatorToCK3.CommonUtils;
45
using ImperatorToCK3.Imperator;
56
using ImperatorToCK3.UnitTests.TestHelpers;
67
using Xunit;
@@ -39,13 +40,14 @@ public void Dispose() {
3940
public void OutputContinueGameJson_canOverwriteReadOnlyFile() {
4041
var continueGamePath = Path.Combine(config.ImperatorDocPath, "continue_game.json");
4142
File.WriteAllText(continueGamePath, "old content");
42-
File.SetAttributes(continueGamePath, FileAttributes.ReadOnly);
43+
SetFileReadOnly(continueGamePath);
4344

4445
var result = (bool)InvokeWorldMethod("OutputContinueGameJson", config)!;
4546

4647
Assert.True(result);
47-
Assert.Contains("\"title\": \"test-save\"", File.ReadAllText(continueGamePath), StringComparison.Ordinal);
48-
Assert.False(File.GetAttributes(continueGamePath).HasFlag(FileAttributes.ReadOnly));
48+
Assert.Contains("\"title\":\t\"test-save\"", File.ReadAllText(continueGamePath), StringComparison.Ordinal);
49+
// New file must be writable on all platforms.
50+
Assert.True(IsFileWritable(continueGamePath));
4951
}
5052

5153
[Fact]
@@ -125,4 +127,31 @@ public void ExtractDynamicCoatsOfArms_restoresBackupFilesAfterCompletion() {
125127
Assert.NotNull(method);
126128
return method!.Invoke(world, args);
127129
}
130+
131+
/// <summary>
132+
/// Marks a file read-only using the appropriate mechanism for the current OS.
133+
/// On Windows this sets <see cref="FileAttributes.ReadOnly"/>; on macOS/Linux
134+
/// it removes the user-write bit via <see cref="UnixFileMode"/>.
135+
/// </summary>
136+
private static void SetFileReadOnly(string filePath) {
137+
if (OperatingSystem.IsWindows()) {
138+
File.SetAttributes(filePath, FileAttributes.ReadOnly);
139+
} else {
140+
var mode = File.GetUnixFileMode(filePath);
141+
File.SetUnixFileMode(filePath, mode & ~UnixFileMode.UserWrite);
142+
}
143+
}
144+
145+
/// <summary>
146+
/// Returns <see langword="true"/> when the current user can write to the file.
147+
/// Uses <see cref="FileHelper.EnsureFileIsWritable"/> indirectly by checking
148+
/// whether the write bit is present rather than relying on
149+
/// <see cref="FileAttributes.ReadOnly"/> which has platform-dependent semantics.
150+
/// </summary>
151+
private static bool IsFileWritable(string filePath) {
152+
if (OperatingSystem.IsWindows()) {
153+
return !File.GetAttributes(filePath).HasFlag(FileAttributes.ReadOnly);
154+
}
155+
return File.GetUnixFileMode(filePath).HasFlag(UnixFileMode.UserWrite);
156+
}
128157
}

‎ImperatorToCK3/CommonUtils/FileHelper.cs‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace ImperatorToCK3.CommonUtils;
77
using System;
88
using Polly;
99
using System.IO;
10+
using System.Linq;
1011
using System.Text;
1112

1213
public static class FileHelper {
@@ -71,6 +72,52 @@ public static void DeleteWithRetries(string filePath) {
7172
}
7273
}
7374

75+
public static void DeleteDirectoryWithRetries(string directoryPath) {
76+
if (string.IsNullOrEmpty(directoryPath) || !Directory.Exists(directoryPath)) {
77+
return;
78+
}
79+
80+
const int maxAttempts = 10;
81+
int currentAttempt = 0;
82+
83+
var policy = Policy
84+
.Handle<IOException>(IsFilesSharingViolation)
85+
.Or<UnauthorizedAccessException>()
86+
.WaitAndRetry(maxAttempts,
87+
sleepDurationProvider: _ => TimeSpan.FromSeconds(1),
88+
onRetry: (_, _, _) => {
89+
currentAttempt++;
90+
Logger.Warn($"Attempt {currentAttempt} to delete directory \"{directoryPath}\" failed.");
91+
Logger.Warn(CloseProgramsHint);
92+
});
93+
94+
try {
95+
policy.Execute(() => {
96+
ResetAttributesRecursively(directoryPath);
97+
Directory.Delete(directoryPath, recursive: true);
98+
});
99+
} catch (IOException ex) when (IsFilesSharingViolation(ex)) {
100+
Logger.Debug(ex.ToString());
101+
throw new UserErrorException($"Failed to delete directory \"{directoryPath}\". {CloseProgramsHint}");
102+
} catch (UnauthorizedAccessException ex) {
103+
Logger.Debug(ex.ToString());
104+
throw new UserErrorException($"Failed to delete directory \"{directoryPath}\": {ex.Message}");
105+
}
106+
}
107+
108+
private static void ResetAttributesRecursively(string directoryPath) {
109+
File.SetAttributes(directoryPath, FileAttributes.Normal);
110+
111+
foreach (var filePath in Directory.EnumerateFiles(directoryPath, "*", SearchOption.AllDirectories)) {
112+
File.SetAttributes(filePath, FileAttributes.Normal);
113+
}
114+
115+
foreach (var subdirectoryPath in Directory.EnumerateDirectories(directoryPath, "*", SearchOption.AllDirectories)
116+
.OrderByDescending(path => path.Length)) {
117+
File.SetAttributes(subdirectoryPath, FileAttributes.Normal);
118+
}
119+
}
120+
74121
// Ensures that the given directory path exists. If a file exists with the
75122
// same name as the desired directory it will be removed first. The method
76123
// retries the creation when a sharing violation occurs, much like the
@@ -134,4 +181,28 @@ public static void MoveWithRetries(string sourceFilePath, string destFilePath) {
134181
throw new UserErrorException($"Failed to move \"{sourceFilePath}\" to \"{destFilePath}\". {CloseProgramsHint}");
135182
}
136183
}
184+
185+
/// <summary>
186+
/// Makes an existing regular file writable by the current user.
187+
/// On Windows this clears the read-only attribute flag; on macOS and Linux
188+
/// it adds the user-write bit directly via the POSIX file mode so that
189+
/// the change takes effect even when the Win32-style attribute mapping
190+
/// does not round-trip correctly on the host OS.
191+
/// Does nothing when the file does not exist or when the path refers to a
192+
/// directory rather than a regular file.
193+
/// </summary>
194+
public static void EnsureFileIsWritable(string filePath) {
195+
if (!File.Exists(filePath)) {
196+
return;
197+
}
198+
199+
if (OperatingSystem.IsWindows()) {
200+
File.SetAttributes(filePath, FileAttributes.Normal);
201+
} else {
202+
var mode = File.GetUnixFileMode(filePath);
203+
if (!mode.HasFlag(UnixFileMode.UserWrite)) {
204+
File.SetUnixFileMode(filePath, mode | UnixFileMode.UserWrite);
205+
}
206+
}
207+
}
137208
}

0 commit comments

Comments
 (0)