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

Remove BinaryFormatter from StateFileBase #6350

Merged
merged 8 commits into from May 24, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions src/Tasks.UnitTests/AssemblyRegistrationCache_Tests.cs
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using Microsoft.Build.Tasks;
using Shouldly;
using Xunit;

namespace Microsoft.Build.UnitTests
Expand All @@ -26,5 +27,24 @@ public void ExerciseCache()
Assert.Equal("foo", assembly);
Assert.Equal("bar", tlb);
}

[Fact]
public void ExerciseCacheSerialization()
{
AssemblyRegistrationCache arc = new();
arc.AddEntry("foo", "bar");
AssemblyRegistrationCache arc2 = null;
using (TestEnvironment env = TestEnvironment.Create())
{
TransientTestFile file = env.CreateFile();
arc.SerializeCache(file.Path, null);
arc2 = StateFileBase.DeserializeCache(file.Path, null, typeof(AssemblyRegistrationCache)) as AssemblyRegistrationCache;
}

arc2._assemblies.Count.ShouldBe(arc._assemblies.Count);
arc2._assemblies[0].ShouldBe(arc._assemblies[0]);
arc2._typeLibraries.Count.ShouldBe(arc._typeLibraries.Count);
arc2._typeLibraries[0].ShouldBe(arc._typeLibraries[0]);
}
}
}
26 changes: 26 additions & 0 deletions src/Tasks.UnitTests/ResolveComReference_Tests.cs
Expand Up @@ -14,6 +14,9 @@
using Microsoft.Build.Tasks;
using Xunit;
using Microsoft.Build.Shared;
using System.IO;
using Microsoft.Build.BackEnd;
using Shouldly;

namespace Microsoft.Build.UnitTests
{
Expand Down Expand Up @@ -57,6 +60,29 @@ public void GetResolvedASsemblyReferenceSpecNotNull()
Assert.NotNull(task.GetResolvedAssemblyReferenceItemSpecs());
}

[Fact]
public void TestSerializationAndDeserialization()
{
ResolveComReferenceCache cache = new("path1", "path2");
cache.componentTimestamps = new()
{
{ "first", DateTime.Now },
{ "second", DateTime.FromBinary(10000) },
};
ResolveComReferenceCache cache2 = null;
using (TestEnvironment env = TestEnvironment.Create())
{
TransientTestFile file = env.CreateFile();
cache.SerializeCache(file.Path, null);
cache2 = StateFileBase.DeserializeCache(file.Path, null, typeof(ResolveComReferenceCache)) as ResolveComReferenceCache;
}

cache2.tlbImpLocation.ShouldBe(cache.tlbImpLocation);
cache2.axImpLocation.ShouldBe(cache.axImpLocation);
cache2.componentTimestamps.Count.ShouldBe(cache.componentTimestamps.Count);
cache2.componentTimestamps["second"].ShouldBe(cache.componentTimestamps["second"]);
}

/*
* Method: CheckComReferenceAttributeVerificationForNameItems
*
Expand Down
55 changes: 43 additions & 12 deletions src/Tasks.UnitTests/ResourceHandling/ResGenDependencies_Tests.cs
Expand Up @@ -6,6 +6,8 @@
using Microsoft.Build.Tasks;
using Microsoft.Build.Shared;
using Xunit;
using Shouldly;
using System;

namespace Microsoft.Build.UnitTests
{
Expand All @@ -16,35 +18,64 @@ sealed public class ResGenDependencies_Tests

public void DirtyCleanScenario(bool useMSBuildResXReader)
{
ResGenDependencies cache = new ResGenDependencies();

ResGenDependencies cache = new();
string resx = CreateSampleResx();
string stateFile = FileUtilities.GetTemporaryFile();

try
{
// A newly created cache is not dirty.
Assert.False(cache.IsDirty);
cache.IsDirty.ShouldBeFalse();

ResGenDependencies.PortableLibraryFile libFile = new("otherFileName");
libFile.outputFiles = new string[] { "first", "second" };
libFile.assemblySimpleName = "simpleName";
libFile.lastModified = DateTime.Now.Subtract(TimeSpan.FromSeconds(10));
cache.portableLibraries.AddDependencyFile("fileName", libFile);

// Writing the file to disk should make the cache clean.
cache.SerializeCache(stateFile, /* Log */ null);
cache.IsDirty.ShouldBeFalse();

// Getting a file that wasn't in the cache is a write operation.
cache.GetResXFileInfo(resx, useMSBuildResXReader);
Assert.True(cache.IsDirty);
cache.IsDirty.ShouldBeTrue();

// Writing the file to disk should make the cache clean.
// Add linkedFiles to further test serialization and deserialization.
cache.resXFiles.dependencies.TryGetValue(resx, out DependencyFile file).ShouldBeTrue();
(file as ResGenDependencies.ResXFile).linkedFiles = new string[] { "third", "fourth" };

// Writing the file to disk should make the cache clean again.
cache.SerializeCache(stateFile, /* Log */ null);
Assert.False(cache.IsDirty);
cache.IsDirty.ShouldBeFalse();

// Deserialize from disk. Result should not be dirty.
cache = ResGenDependencies.DeserializeCache(stateFile, true, /* Log */ null);
Assert.False(cache.IsDirty);
ResGenDependencies cache2 = ResGenDependencies.DeserializeCache(stateFile, true, /* Log */ null);
cache2.IsDirty.ShouldBeFalse();

// Validate that serialization worked
ResGenDependencies.PortableLibraryFile portableLibrary = cache.portableLibraries.GetDependencyFile("fileName") as ResGenDependencies.PortableLibraryFile;
ResGenDependencies.PortableLibraryFile portableLibrary2 = cache2.portableLibraries.GetDependencyFile("fileName") as ResGenDependencies.PortableLibraryFile;
portableLibrary2.filename.ShouldBe(portableLibrary.filename);
portableLibrary2.exists.ShouldBe(portableLibrary.exists);
portableLibrary2.assemblySimpleName.ShouldBe(portableLibrary.assemblySimpleName);
portableLibrary2.lastModified.ShouldBe(portableLibrary.lastModified);
portableLibrary2.outputFiles.Length.ShouldBe(portableLibrary.outputFiles.Length);
portableLibrary2.outputFiles[1].ShouldBe(portableLibrary.outputFiles[1]);
ResGenDependencies.ResXFile resX = cache.resXFiles.GetDependencyFile(resx) as ResGenDependencies.ResXFile;
ResGenDependencies.ResXFile resX2 = cache2.resXFiles.GetDependencyFile(resx) as ResGenDependencies.ResXFile;
resX2.filename.ShouldBe(resX.filename);
resX2.lastModified.ShouldBe(resX.lastModified);
resX2.linkedFiles.Length.ShouldBe(resX.linkedFiles.Length);
resX2.linkedFiles[1].ShouldBe(resX.linkedFiles[1]);

// Asking for a file that's in the cache should not dirty the cache.
cache.GetResXFileInfo(resx, useMSBuildResXReader);
Assert.False(cache.IsDirty);
cache2.GetResXFileInfo(resx, useMSBuildResXReader);
cache2.IsDirty.ShouldBeFalse();

// Changing UseSourcePath to false should dirty the cache.
cache.UseSourcePath = false;
Assert.True(cache.IsDirty);
cache2.UseSourcePath = false;
cache2.IsDirty.ShouldBeTrue();
}
finally
{
Expand Down
6 changes: 2 additions & 4 deletions src/Tasks/AssemblyRegistrationCache.cs
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections.Generic;
using Microsoft.Build.Shared;

Expand All @@ -10,18 +9,17 @@ namespace Microsoft.Build.Tasks
/// <remarks>
/// This class is a caching mechanism for the Register/UnregisterAssembly task to keep track of registered assemblies to clean up
/// </remarks>
[Serializable()]
internal sealed class AssemblyRegistrationCache : StateFileBase
{
/// <summary>
/// The list of registered assembly files.
/// </summary>
private readonly List<string> _assemblies = new List<string>();
internal List<string> _assemblies = new List<string>();

/// <summary>
/// The list of registered type library files.
/// </summary>
private readonly List<string> _typeLibraries = new List<string>();
internal List<string> _typeLibraries = new List<string>();

/// <summary>
/// The number of entries in the state file
Expand Down
12 changes: 5 additions & 7 deletions src/Tasks/Dependencies.cs
@@ -1,32 +1,30 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System;
using System.Collections;
using System.Collections.Generic;

namespace Microsoft.Build.Tasks
{
/// <summary>
/// Represents a cache of inputs to a compilation-style task.
/// </summary>
/// <remarks>On-disk serialization format, don't change field names or types or use readonly.</remarks>
[Serializable]
internal class Dependencies
{
/// <summary>
/// Hashtable of other dependency files.
/// Key is filename and value is DependencyFile.
/// </summary>
private Hashtable dependencies = new Hashtable();
internal Dictionary<string, DependencyFile> dependencies = new();

/// <summary>
/// Look up a dependency file. Return null if its not there.
/// Look up a dependency file. Return null if it isn't there.
/// </summary>
/// <param name="filename"></param>
/// <returns></returns>
internal DependencyFile GetDependencyFile(string filename)
{
return (DependencyFile)dependencies[filename];
dependencies.TryGetValue(filename, out DependencyFile file);
return file;
}

/// <summary>
Expand Down
13 changes: 7 additions & 6 deletions src/Tasks/DependencyFile.cs
Expand Up @@ -12,20 +12,17 @@ namespace Microsoft.Build.Tasks
/// <remarks>
/// Represents a single input to a compilation-style task.
/// Keeps track of timestamp for later comparison.
///
/// On-disk serialization format, don't change field names or types or use readonly.
/// </remarks>
[Serializable]
internal class DependencyFile
{
// Filename
private string filename;
internal string filename;

// Date and time the file was last modified
private DateTime lastModified;
internal DateTime lastModified;

// Whether the file exists or not.
private bool exists = false;
internal bool exists = false;

/// <summary>
/// The name of the file.
Expand Down Expand Up @@ -70,6 +67,10 @@ internal DependencyFile(string filename)
}
}

internal DependencyFile()
{
}

/// <summary>
/// Checks whether the file has changed since the last time a timestamp was recorded.
/// </summary>
Expand Down
28 changes: 16 additions & 12 deletions src/Tasks/ResGenDependencies.cs
Expand Up @@ -22,18 +22,17 @@ namespace Microsoft.Build.Tasks
///
/// This is an on-disk serialization format, don't change field names or types or use readonly.
/// </remarks>
[Serializable]
internal sealed class ResGenDependencies : StateFileBase
{
/// <summary>
/// The list of resx files.
/// </summary>
private Dependencies resXFiles = new Dependencies();
internal Dependencies resXFiles = new Dependencies();

/// <summary>
/// A list of portable libraries and the ResW files they can produce.
/// </summary>
private Dependencies portableLibraries = new Dependencies();
internal Dependencies portableLibraries = new Dependencies();

/// <summary>
/// A newly-created ResGenDependencies is not dirty.
Expand All @@ -47,7 +46,7 @@ internal sealed class ResGenDependencies : StateFileBase
/// If this is NULL then we use the directory in which the .resx is in (that should always
/// be the default!)
/// </summary>
private string baseLinkedFileDirectory;
internal string baseLinkedFileDirectory;

internal string BaseLinkedFileDirectory
{
Expand Down Expand Up @@ -93,8 +92,7 @@ internal bool UseSourcePath
internal ResXFile GetResXFileInfo(string resxFile, bool useMSBuildResXReader)
{
// First, try to retrieve the resx information from our hashtable.
var retVal = (ResXFile)resXFiles.GetDependencyFile(resxFile);
if (retVal == null)
if (resXFiles.GetDependencyFile(resxFile) is not ResXFile retVal || retVal == null)
Copy link
Member

Choose a reason for hiding this comment

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

I don't feel like this is clearer than switching the cast to an as.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's probably better, thanks. This wasn't about clarity but correctness—the cast was failing and throwing an exception because it wasn't a ResXFile. That shouldn't happen in practice, and it's now fixed (see previous comment), but did cause problems for a bit.

{
// Ok, the file wasn't there. Add it to our cache and return it to the caller.
retVal = AddResxFile(resxFile, useMSBuildResXReader);
Expand Down Expand Up @@ -188,11 +186,10 @@ internal static ResGenDependencies DeserializeCache(string stateFile, bool useSo
///
/// This is an on-disk serialization format, don't change field names or types or use readonly.
/// </remarks>
[Serializable]
internal sealed class ResXFile : DependencyFile
{
// Files contained within this resx file.
private string[] linkedFiles;
internal string[] linkedFiles;

internal string[] LinkedFiles => linkedFiles;

Expand All @@ -209,6 +206,10 @@ internal ResXFile(string filename, string baseLinkedFileDirectory, bool useMSBui
}
}

internal ResXFile()
{
}

/// <summary>
/// Given a .RESX file, returns all the linked files that are referenced within that .RESX.
/// </summary>
Expand Down Expand Up @@ -281,12 +282,15 @@ private static string[] GetLinkedFiles(string filename, string baseLinkedFileDir
///
/// This is an on-disk serialization format, don't change field names or types or use readonly.
/// </remarks>
[Serializable]
internal sealed class PortableLibraryFile : DependencyFile
{
private string[] outputFiles;
private string neutralResourceLanguage;
private string assemblySimpleName;
internal string[] outputFiles;
internal string neutralResourceLanguage;
internal string assemblySimpleName;

internal PortableLibraryFile()
{
}

internal PortableLibraryFile(string filename)
: base(filename)
Expand Down