-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Move version check earlier #6674
Changes from 2 commits
7ddd2d2
c38d9a7
aa75790
d71e2a5
e4d4e0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -77,8 +77,17 @@ internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelp | |||||
using (FileStream s = File.OpenRead(stateFile)) | ||||||
{ | ||||||
using var translator = BinaryTranslator.GetReadTranslator(s, buffer: null); | ||||||
|
||||||
byte version = 0; | ||||||
translator.Translate(ref version); | ||||||
// If retVal is still null or the version is wrong, log a message not a warning. This could be a valid cache with the wrong version preventing correct deserialization. | ||||||
// For the latter case, internals may be unexpectedly null. | ||||||
if (retVal == null || version != CurrentSerializationVersion) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. retval will always be null here, right?
Suggested change
|
||||||
{ | ||||||
log.LogMessageFromResources("General.CouldNotReadStateFileMessage", stateFile, log.FormatResourceString("General.IncompatibleStateFileType")); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it continues to make sense for this to be a quiet message. But do we have a test in place that this is not the case for freshly-built-everything using the SDK cache? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We do not. If I remember correctly, we decided it would be too hard for some reason. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we at least get it added to the manual SDK tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that sounds like a good idea to me. @marcpopMSFT, agree? If so, I can write up some (very detailed) instructions on testing it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please reach out to Richa with the scenario you want covered. The difficulty with fresh machine tests as you can only have one. |
||||||
return null; | ||||||
} | ||||||
|
||||||
var constructors = requiredReturnType.GetConstructors(); | ||||||
foreach (var constructor in constructors) | ||||||
{ | ||||||
|
@@ -88,18 +97,8 @@ internal static StateFileBase DeserializeCache(string stateFile, TaskLoggingHelp | |||||
retVal = constructor.Invoke(new object[] { translator }) as StateFileBase; | ||||||
} | ||||||
} | ||||||
|
||||||
// If retVal is still null or the version is wrong, log a message not a warning. This could be a valid cache with the wrong version preventing correct deserialization. | ||||||
// For the latter case, internals may be unexpectedly null. | ||||||
if (retVal == null || version != CurrentSerializationVersion) | ||||||
{ | ||||||
// When upgrading to Visual Studio 2008 and running the build for the first time the resource cache files are replaced which causes a cast error due | ||||||
// to a new version number on the tasks class. "Unable to cast object of type 'Microsoft.Build.Tasks.SystemState' to type 'Microsoft.Build.Tasks.StateFileBase". | ||||||
// If there is an invalid cast, a message rather than a warning should be emitted. | ||||||
log.LogMessageFromResources("General.CouldNotReadStateFileMessage", stateFile, log.FormatResourceString("General.IncompatibleStateFileType")); | ||||||
return null; | ||||||
} | ||||||
else if (!requiredReturnType.IsInstanceOfType(retVal)) | ||||||
|
||||||
if (!requiredReturnType.IsInstanceOfType(retVal)) | ||||||
{ | ||||||
log.LogMessageFromResources("General.CouldNotReadStateFileMessage", stateFile, | ||||||
log.FormatResourceString("General.IncompatibleStateFileType")); | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update comment