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

Question mode usage feedback updates. #9155

Merged
merged 13 commits into from
Nov 18, 2023
53 changes: 53 additions & 0 deletions src/Tasks.UnitTests/ResourceHandling/GenerateResource_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,59 @@ public void ForceOutOfDateLinked(bool usePreserialized)
}
}

[Fact]
public void QuestionOutOfDateByDeletion()
{
var folder = _env.CreateFolder();
string resxFileInput = Utilities.WriteTestResX(false, null, null, _env.CreateFile(folder, ".resx").Path);
TaskItem stateFile = new TaskItem(_env.GetTempFile(".cache").Path);
ITaskItem[] sources = new ITaskItem[] { new TaskItem(resxFileInput) };
ITaskItem[] output;

GenerateResource t1 = Utilities.CreateTask(_output);
t1.Sources = sources;
t1.StateFile = stateFile;
Utilities.ExecuteTask(t1);

Utilities.AssertLogContainsResource(t1, "GenerateResource.OutputDoesntExist", t1.OutputResources[0].ItemSpec);

output = t1.OutputResources;

// Run again to ensure all files are up to date.
GenerateResource t2 = Utilities.CreateTask(_output);
t2.Sources = sources;
t2.StateFile = stateFile;
t2.FailIfNotIncremental = true;
Utilities.ExecuteTask(t2);

// Delete the file and verify that FailIfNotIncremental will print the missing file
GenerateResource t3 = Utilities.CreateTask(_output);
t3.StateFile = stateFile;
t3.Sources = sources;
t3.FailIfNotIncremental = true;

// Delete the output
File.Delete(output[0].ItemSpec);

t3.Execute().ShouldBeFalse();

Utilities.AssertLogContainsResource(t3, "GenerateResource.ProcessingFile", sources[0].ItemSpec, output[0].ItemSpec);

GenerateResource t4 = Utilities.CreateTask(_output);
t4.Sources = sources;
t4.StateFile = stateFile;
Utilities.ExecuteTask(t4);

Utilities.AssertLogContainsResource(t4, "GenerateResource.OutputDoesntExist", t4.OutputResources[0].ItemSpec);

// Run again to ensure all files are up to date.
GenerateResource t5 = Utilities.CreateTask(_output);
t5.Sources = sources;
t5.StateFile = stateFile;
t5.FailIfNotIncremental = true;
Utilities.ExecuteTask(t5);
}

[Theory]
[InlineData(false, false)]
[InlineData(false, true)]
Expand Down
4 changes: 2 additions & 2 deletions src/Tasks.UnitTests/Touch_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ public void QuestionTouchNonExistingAlwaysCreate()

bool success = Execute(t);

Assert.False(success);
Assert.True(success);

Assert.Contains(
String.Format(AssemblyResources.GetString("Touch.CreatingFile"), mynonexisting_txt, "AlwaysCreate"),
Expand All @@ -401,7 +401,7 @@ public void QuestionTouchExisting()

bool success = Execute(t);

Assert.False(success);
Assert.True(success);

Assert.Contains(
String.Format(AssemblyResources.GetString("Touch.Touching"), myexisting_txt),
Expand Down
12 changes: 5 additions & 7 deletions src/Tasks/Delete.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,15 +123,13 @@ public override bool Execute()
{
if (FailIfNotIncremental)
{
Log.LogErrorFromResources("Delete.DeletingFile", file.ItemSpec);
Log.LogWarningFromResources("Delete.DeletingFile", file.ItemSpec);
}
else
{
// Do not log a fake command line as well, as it's superfluous, and also potentially expensive
Log.LogMessageFromResources(MessageImportance.Normal, "Delete.DeletingFile", file.ItemSpec);

File.Delete(file.ItemSpec);
}
// Do not log a fake command line as well, as it's superfluous, and also potentially expensive
Log.LogMessageFromResources(MessageImportance.Normal, "Delete.DeletingFile", file.ItemSpec);
yuehuang010 marked this conversation as resolved.
Show resolved Hide resolved

File.Delete(file.ItemSpec);
}
else
{
Expand Down
36 changes: 8 additions & 28 deletions src/Tasks/FileIO/WriteLinesToFile.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,10 @@ public class WriteLinesToFile : TaskExtension, IIncrementalTask
/// <summary>
/// Question whether this task is incremental.
/// </summary>
/// <remarks>When question is true, then this task would not write to disk. If CanBeIncremental is true, then error out.</remarks>
/// <remarks>When question is true, then error out if WriteOnlyWhenDifferent would have
/// written to the file.</remarks>
public bool FailIfNotIncremental { get; set; }

public bool CanBeIncremental => WriteOnlyWhenDifferent;
yuehuang010 marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Execute the task.
/// </summary>
Expand All @@ -69,7 +68,7 @@ public override bool Execute()
// do not return if Lines is null, because we may
// want to delete the file in that case
StringBuilder buffer = new StringBuilder();
if (Lines != null && (!FailIfNotIncremental || WriteOnlyWhenDifferent))
if (Lines != null)
{
foreach (ITaskItem line in Lines)
{
Expand Down Expand Up @@ -131,36 +130,17 @@ public override bool Execute()
MSBuildEventSource.Log.WriteLinesToFileUpToDateStop(File.ItemSpec, false);
}

if (FailIfNotIncremental)
{
if (Lines?.Length > 0)
{
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorReadingFile", File.ItemSpec);
return false;
}
}
else
{
System.IO.File.WriteAllText(File.ItemSpec, contentsAsString, encoding);
}
System.IO.File.WriteAllText(File.ItemSpec, contentsAsString, encoding);
}
else
{
if (FailIfNotIncremental && Lines?.Length > 0)
if (WriteOnlyWhenDifferent)
JanKrivanek marked this conversation as resolved.
Show resolved Hide resolved
{
Log.LogErrorWithCodeFromResources("WriteLinesToFile.ErrorOrWarning", File.ItemSpec, string.Empty);
return false;
Log.LogMessageFromResources(MessageImportance.Normal, "WriteLinesToFile.UnusedWriteOnlyWhenDifferent", File.ItemSpec);
}
else
{
if (WriteOnlyWhenDifferent)
{
Log.LogMessageFromResources(MessageImportance.Normal, "WriteLinesToFile.UnusedWriteOnlyWhenDifferent", File.ItemSpec);
}

Directory.CreateDirectory(directoryPath);
System.IO.File.AppendAllText(File.ItemSpec, buffer.ToString(), encoding);
}
Directory.CreateDirectory(directoryPath);
System.IO.File.AppendAllText(File.ItemSpec, buffer.ToString(), encoding);
}
}
catch (Exception e) when (ExceptionHandling.IsIoRelatedException(e))
Expand Down
16 changes: 13 additions & 3 deletions src/Tasks/GenerateResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,15 @@ public override bool Execute()
}
else if (FailIfNotIncremental)
{
Log.LogErrorFromResources("GenerateResource.OutOfDate");
int maxCount = Math.Min(inputsToProcess.Count, outputsToProcess.Count);
maxCount = Math.Min(maxCount, 5); // Limit to just 5

for (int index = 0; index < maxCount; index++)
{
Log.LogErrorFromResources("GenerateResource.ProcessingFile", inputsToProcess[index], outputsToProcess[index]);
yuehuang010 marked this conversation as resolved.
Show resolved Hide resolved
}

return false;
}
else
{
Expand Down Expand Up @@ -3605,6 +3613,7 @@ private void ReadTextResources(ReaderInfo reader, String fileName)
name.Length--;
}
ch = sr.Read(); // move past =

// If it exists, move past the first space after the equals sign.
if (ch == ' ')
{
Expand Down Expand Up @@ -3746,10 +3755,11 @@ private void WriteResources(ReaderInfo reader,
// In that case, the first time we catch an exception indicating that the XML written to disk is malformed,
// specifically an InvalidOperationException: "Token EndElement in state Error would result in an invalid XML document."
try { writer.Dispose(); }
catch (Exception) { } // We agressively catch all exception types since we already have one we will throw.
catch (Exception) { } // We aggressively catch all exception types since we already have one we will throw.

// The second time we catch the out of disk space exception.
try { writer.Dispose(); }
catch (Exception) { } // We agressively catch all exception types since we already have one we will throw.
catch (Exception) { } // We aggressively catch all exception types since we already have one we will throw.
throw capturedException; // In the event of a full disk, this is an out of disk space IOException.
}
}
Expand Down
13 changes: 4 additions & 9 deletions src/Tasks/Touch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,7 @@ private bool TouchFile(
{
if (FailIfNotIncremental)
{
Log.LogErrorFromResources("Touch.CreatingFile", file, "AlwaysCreate");
return false;
Log.LogWarningFromResources("Touch.CreatingFile", file, "AlwaysCreate");
}
else
{
Expand All @@ -222,17 +221,13 @@ private bool TouchFile(
}
}

// Ignore touching the disk when FailIfNotIncremental.
if (FailIfNotIncremental)
{
Log.LogErrorFromResources("Touch.Touching", file);
return false;
}
else
{
Log.LogMessageFromResources(messageImportance, "Touch.Touching", file);
Log.LogWarningFromResources("Touch.Touching", file);
}

Log.LogMessageFromResources(messageImportance, "Touch.Touching", file);
yuehuang010 marked this conversation as resolved.
Show resolved Hide resolved

// If the file is read only then we must either issue an error, or, if the user so
// specified, make the file temporarily not read only.
bool needToRestoreAttributes = false;
Expand Down
Loading