Skip to content

[NoMerge] StringValues convert to Empty Array#330

Closed
benaadams wants to merge 3 commits into
dotnet:masterfrom
benaadams:sv-empty-array
Closed

[NoMerge] StringValues convert to Empty Array#330
benaadams wants to merge 3 commits into
dotnet:masterfrom
benaadams:sv-empty-array

Conversation

@benaadams
Copy link
Copy Markdown
Member

@Tratcher
Copy link
Copy Markdown
Member

Heads up, the original change was reverted. We'll reintroduce it more cautiously by doing full universe builds before merging.

@Tratcher
Copy link
Copy Markdown
Member

Tratcher commented Mar 21, 2018

@benaadams this change matches our best theory on how to mitigate the risks of the other changes. Can you cherry pick your prior commits back into this PR?

Have you run a full universe build before?

@benaadams
Copy link
Copy Markdown
Member Author

Have you run a full universe build before?

Never :) Sounds big...

@benaadams
Copy link
Copy Markdown
Member Author

Will have a browse at https://github.com/aspnet/Universe

@Tratcher
Copy link
Copy Markdown
Member

The gist is that you make your changes in your branch and push them. Then you clone universe including submodules, update the Common submodule to point at your branch/commit, and then build the root universe.

@benaadams
Copy link
Copy Markdown
Member Author

I now know git modules
200w_d

@benaadams
Copy link
Copy Markdown
Member Author

Know what I need installed for IISIntegration?

 >>> dotnet.exe msbuild @C:\GitHub\Universe\modules\IISIntegration\artifacts\logs\msbuild.rsp @C:\GitHub\Universe\modules\IISIntegration\artifacts\logs\msbuild.logger.rsp
  IISIntegration        | Build started.
  IISIntegration        | C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-15745\modules\KoreBuild.Tasks\module.targets(140,5): error :
 Could not find an installation of Visual Studio that satisifies the specified requirements in 
C:\GitHub\Universe\modules\IISIntegration\/korebuild.json. 
See https://docs.microsoft.com/en-us/visualstudio/install/workload-component-id-vs-community for more details on any missing components.
  IISIntegration        |
  IISIntegration        | Build FAILED.
  IISIntegration        |
  IISIntegration        | C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-15745\modules\KoreBuild.Tasks\module.targets(140,5): error : Could not find an installation of Visual Studio that satisifies the specified requirements in C:\GitHub\Universe\modules\IISIntegration\/korebuild.json. See https://docs.microsoft.com/en-us/visualstudio/install/workload-component-id-vs-community for more details on any missing components.
  IISIntegration        |     0 Warning(s)
  IISIntegration        |     1 Error(s)
  IISIntegration        |
  IISIntegration        | Time Elapsed 00:00:00.30
  dotnet.exe failed with exit code: 1
  At C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-15745\scripts\common.psm1:11 char:9
  +         throw "$cmdName failed with exit code: $exitCode"
  +         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      + CategoryInfo          : OperationStopped: (dotnet.exe failed with exit code: 1:String) [], RuntimeException
      + FullyQualifiedErrorId : dotnet.exe failed with exit code: 1

@benaadams
Copy link
Copy Markdown
Member Author

Ah, ok

"requiredWorkloads": [
  "Microsoft.VisualStudio.Component.VC.Tools.x86.x64",
  "Microsoft.VisualStudio.ComponentGroup.NativeDesktop.Win81",
  "Microsoft.VisualStudio.Component.VC.ATL",
  "Microsoft.VisualStudio.Component.Windows10SDK.15063.Desktop"
]

@benaadams
Copy link
Copy Markdown
Member Author

Is there a way to get Universe builds to log? When it goes wrong I just get a thousand lines of red, but can't scroll back to see what the initial error was as it fills the console buffer pretty fast

@Tratcher
Copy link
Copy Markdown
Member

Pipe to file?

@halter73
Copy link
Copy Markdown
Member

Use tee so you can still see the output while piping to a file.

@benaadams
Copy link
Copy Markdown
Member Author

Piror to change, Think this is just it being grumpy

Build FAILED.

warning : Skipping template tests because InstallSharedRuntimeFromPreviousBuild != 'true'. [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error : Tests failed for the following repos: [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - AADIntegration [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Identity [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - AzureIntegration [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Scaffolding [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - MvcPrecompilation [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - MusicStore [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Razor [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]

Need to find what fails with change

@benaadams
Copy link
Copy Markdown
Member Author

A lot of test errors seem to be \n\r vs \r

Build FAILED.

warning : Skipping template tests because InstallSharedRuntimeFromPreviousBuild != 'true'. [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error : Tests failed for the following repos: [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Mvc [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - AADIntegration [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - AzureIntegration [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Identity [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Scaffolding [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - MvcPrecompilation [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - MusicStore [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - Razor [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]
 error :  - HttpSysServer [C:\Users\thund\.dotnet\buildtools\korebuild\2.1.0-preview3-17001\KoreBuild.proj]

Mvc and HttpSysServer are new; will prepare fixes

@benaadams
Copy link
Copy Markdown
Member Author

@Tratcher HttpSysServer is inconsistent between known and unknown headers and empty/nulls; this change will bring them closer, but not all the way..?

public static TheoryData<string, StringValues, StringValues> NullHeaderData
{
    get
    {
        var dataset = new TheoryData<string, StringValues, StringValues>();

        // Unknown headers
        dataset.Add("NullString", (string)null, (string)null);
        dataset.Add("EmptyString", "", "");
        dataset.Add("NullStringArray", new string[] { null }, "");
        dataset.Add("EmptyStringArray", new string[] { "" }, "");
        dataset.Add("MixedStringArray", new string[] { null, "" }, new string[] { "", "" });
        // Known headers
        dataset.Add("Location", (string)null, (string)null);
        dataset.Add("Location", "", (string)null);
        dataset.Add("Location", new string[] { null }, (string)null);
        dataset.Add("Location", new string[] { "" }, (string)null);
        dataset.Add("Location", new string[] { "a" }, "a");
        dataset.Add("Location", new string[] { null, "" }, (string)null);
        dataset.Add("Location", new string[] { null, "", "a", "b" }, new string[] { "a", "b" });

        return dataset;
    }
}

@benaadams
Copy link
Copy Markdown
Member Author

Actually with aspnet/HttpAbstractions#1010 it might bring them completely inline

@benaadams
Copy link
Copy Markdown
Member Author

Except dropping nulls from arrays that contain values also; which it does for known headers (backed that change out in the previous PR)

@benaadams benaadams changed the title StringValues convert to Empty Array [NoMerge] StringValues convert to Empty Array Mar 24, 2018
@natemcmaster natemcmaster changed the base branch from dev to master July 2, 2018 17:44
@davidfowl davidfowl closed this Sep 26, 2018
natemcmaster pushed a commit that referenced this pull request Nov 7, 2018
* Ensures our build stays clean of NuGet warnings
@ghost ghost locked as resolved and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants