aolszowka / MsBuildToCCNetvNext

A Next Generation MsBuildToCCNet Logger that is Multi-Processor-Aware
GNU General Public License v3.0
0 stars 0 forks source link

After updating to VS 17.6 this error started. #1

Open mikeacu opened 1 year ago

mikeacu commented 1 year ago

System.ArgumentNullException: Value cannot be null. Parameter name: source at System.Linq.Enumerable.Any[TSource](IEnumerable1 source, Func2 predicate) at MsBuildToCCNetvNext.Utilities.SanitizeMessageForXml(String input) at MsBuildToCCNetvNext.Message.get_XmlFragement() at MsBuildToCCNetvNext.Project.get_XmlFragment() at MsBuildToCCNetvNext.MsBuildToCCNetvNext.Shutdown() at Microsoft.Build.BackEnd.Logging.LoggingService.ShutdownLogger(ILogger logger)

It was determined that msbuild is now sometimes posting null messages.

aolszowka commented 1 year ago

Great to hear again from you Mike; I'd be happy to take a PR that resolves the issue.

I'd probably suggest adding a check like this:


        internal static string SanitizeMessageForXml(string input)
        {
            string sanitizedMsg = input;
            if(string.IsNullOrEmpty(sanitizedMsg)) {
                sanitizedMsg = string.Empty;
            }
            else if (MessageNeedsSanitation(input))
            {
                string invalidXmlCharactersRemoved =
                            new string(input.AsEnumerable().Where(currentChar => XmlConvert.IsXmlChar(currentChar)).ToArray());
                sanitizedMsg =
                    string.Format("WARNING This message contained invalid XML character(s) which have been removed: {0}", invalidXmlCharactersRemoved);
            }

            return sanitizedMsg;
        }

This should still just build with lock-stock-standard Visual Studio

mikeacu commented 1 year ago

Hey Ace!

I had originally tried the change below but ended up modifying message.cs. But I’m ok either way. I’m cool with creating a branch and PR too.

public Message(BuildMessageEventArgs e) { if (e == null) { throw new ArgumentNullException(nameof(e)); } this.Text = e.Message ?? string.Empty; this.Importance = e.Importance; }

From: Ace Olszowka @.> Sent: Friday, June 23, 2023 10:42 AM To: aolszowka/MsBuildToCCNetvNext @.> Cc: Mike Adams @.>; Author @.> Subject: [External] Re: [aolszowka/MsBuildToCCNetvNext] After updating to VS 17.6 this error started. (Issue #1)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

Great to hear again from you Mike; I'd be happy to take a PR that resolves the issue.

I'd probably suggest adding a check like this:

    internal static string SanitizeMessageForXml(string input)

    {

        string sanitizedMsg = input;

        if(string.IsNullOrEmpty(sanitizedMsg)) {

            sanitizedMsg = string.Empty;

        }

        else if (MessageNeedsSanitation(input))

        {

            string invalidXmlCharactersRemoved =

                        new string(input.AsEnumerable().Where(currentChar => XmlConvert.IsXmlChar(currentChar)).ToArray());

            sanitizedMsg =

                string.Format("WARNING This message contained invalid XML character(s) which have been removed: {0}", invalidXmlCharactersRemoved);

        }

        return sanitizedMsg;

    }

This should still just build with lock-stock-standard Visual Studio

— Reply to this email directly, view it on GitHubhttps://github.com/aolszowka/MsBuildToCCNetvNext/issues/1#issuecomment-1604535381, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOX3UZ34D3CODCHGOWUPTO3XMXBOHANCNFSM6AAAAAAZRZFNZI. You are receiving this because you authored the thread.Message ID: @.***>


DISCLAIMER: This message is confidential, intended only for the named recipient(s) and may contain information that is privileged or exempt from disclosure under applicable law. If you are not the intended recipient(s), you are notified that the dissemination, distribution or copying of this message is strictly prohibited. If you receive this message in error, or are not the named recipient(s), please notify the sender at either the e-mail address or by calling the telephone number associated with this transmission. Please delete this e-mail from your computer (or discard this fax). Thank You.

aolszowka commented 1 year ago

I think it comes down to what you want the behavior of the logger to be in that scenario; I believe with your code above it will throw and still die in the logger right? If you're seeing null messages you probably either want to just log a blank and continue on, or even just squelch the message.

Did you have a particular path in mind?

mikeacu commented 1 year ago

Preferably I think we just want to squelch the nulls as they have no value in the logs. As a quick workaround I just changed CC to use the default mslogger. Its not pretty but it got us back working.

aolszowka commented 1 year ago

IIRC the reason we are using the custom logger is for the XLT Transform to work on the build output (you're probably going to be missing Errors, Warnings, and Info Messages). If you've moved away from that no big deal, but if you're still using it the default logger doesn't work. Its also even worse when building in Parallel.

If you want to just squelch the null messages you'll need to change this section:

https://github.com/aolszowka/MsBuildToCCNetvNext/blob/5dab036a03d755eaac9d4bd1241238bad3786cec/MsBuildToCCNetvNext/MsBuildToCCNetvNext.cs#L257C9-L281C10

To do something like (obviously repeated for each of those sections and with the correct type):

if(String.IsNull(e.Message)) {
    // Do nothing
}
else {
    Project currentProject = this.GetOrCreateAssociatedProject(e.ProjectFile);
    currentProject.Add(new Error(e));
}
mikeacu commented 1 year ago

Yeah I know we are missing data in the logs. Luckily we had only updated one of the CC builds with VS 17.6 when we discovered the issue. I'll give that a try and post a pr later today. Thanks.

mikeacu commented 1 year ago

Ace that last suggestion worked great. I was unable to create a remote branch and subsequent PR.

remote: Permission to aolszowka/MsBuildToCCNetvNext.git denied to mikeacu.

aolszowka commented 1 year ago

@mikeacu Its because you are attempting to edit my Git Repository, to contribute on GitHub you need to hit Fork make the changes in your version of the project and then submit a PR upstream. This walks you through it pretty well: https://docs.github.com/en/get-started/quickstart/contributing-to-projects