-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[release/2.1] Correct and streamline internal builds #30729
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
Conversation
Purely infrastructure and therefore |
Any objection to |
Baselines: https://dev.azure.com/dnceng/internal/_build/results?buildId=1026812 (internal build of #30639 containing some overlapping commits to this PR) and https://dev.azure.com/dnceng/public/_build/results?buildId=1026843 (public build of this PR) did run tests. Unfortunately, https://dev.azure.com/dnceng/internal/_build/results?buildId=1026844 (internal build of this PR) also run tests. Suspect |
New sample build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1026905 |
Latest internal build: https://dev.azure.com/dnceng/internal/_build/results?buildId=1026917 |
Thanks! |
No objection to |
@wtgodbe what are the SharedFx-{platform}.ProjectImports.zip files e.g. https://dev.azure.com/dnceng/_apis/resources/Containers/6457484/artifacts-Linux-Musl-SharedFx?itemPath=artifacts-Linux-Musl-SharedFx%2Flogs%2FSharedFx-linux-musl-x64.ProjectImports.zip for❔ I downloaded one and it seems to contain all files from the repo. |
Good question - I have no idea. I don't see that artifact in the last official build of release/2.1: https://dev.azure.com/dnceng/internal/_build/results?buildId=1005457&view=artifacts&pathAsName=false&type=publishedArtifacts |
Do not merge proactively. I want to double-check https://dev.azure.com/dnceng/internal/_build/results?buildId=1030342 |
- correct manual internal builds - need to explicitly set `$(BuildNumber)` property in all build steps - rename `$(BuildScriptArgs)` to avoid circular reference - skip tests by default in internal non-PR builds nits: - include new `$(BuildNumberArg)` in `$(SharedFxArgs)` - run all test jobs in internal pull requests
- however test\SharedFx.UnitTests\SharedFx.UnitTests.csproj always runs as part of `BuildSharedFx` target
- download will usually fail repeatedly if it fails even once - previous maximum duration (105 min.) was greater than the job timeout
- when `!$(ValidateBaseline)`, previous runtimes do not exist
550a362
to
64b7204
Compare
Official builds still hit a lot of NU5123 errors with
@rrelyea what suppresses these errors❔ I ask because the package layout seems to be consistent between e.g. runtime.linux-musl-x64.Microsoft.AspNetCore.All.2.1.17-servicing-31324.symbols.nupkg and Microsoft.AspNetCore.App.Runtime.linux-musl-arm64.3.1.14.symbols.nupkg but NU5123 don't show up in our release/3.1 builds and we don't mention "NU5123" in our release/3.1 branch. Alternatively, @tmat is there another (shorter) way we could name the |
I'm having trouble getting to the root cause of some warnings and errors in internal 2.1.x builds such as https://dev.azure.com/dnceng/internal/_build/results?buildId=1051784
Any help much appreciated❕ |
I believe that Anton recently updated the DiaSymReader to resolve some arm64-related issues but this thread mostly seems to refer to the |
@trylek thanks for your response. Has |
We certainly don't plan to "backport Crossgen2" to the servicing branches, it would be next to impossible and I believe there's no reason in "rewriting history". It's not like Crossgen1 committed a heinous crime and any mention of it has to be erased from everywhere, we just wrote a newer version and we're switching over to it in the releases to come. Please keep in mind that Crossgen1 / 2 is part of the .NET SDK, it's not an "external" compiler or build tool in the sense of being released independently (like Roslyn, MSVC, or cmake) where we need to update to newer versions once in a while even in the servicing branches. |
|
And for your other question, I don't see any recent semantic changes to Crossgen in the 2.1 branch, please see here: https://github.com/dotnet/coreclr/commits/release/2.1/src I believe it's been mostly infra goo and bits of mostly managed stuff unrelated to the Crossgen compiler at least for a year, maybe more. |
I don't know anything about the history of Microsoft.CodeAnalysis and don't even know where the 2.1.x version of that assembly or package is / was built. We could exclude it from our /fyi we do the /cc @jaredpar in case something has changed and @pranavkm for awareness. |
Thanks Anton, you're right, I misread the error message. For Microsoft.CodeAnalysis, I see that the current "live" version in the runtime repo indeed uses pinvokes against DiaSymReader. This would constitute an important change if previously Microsoft.CodeAnalysis had been using dynamic resolution of DiaSymReader methods (LoadLibraryEx & GetProcAddress & Marshal.GetDelegateForFunctionPointer. I don't know if such transformation happened in the past. Why would have we changed the version of Microsoft.CodeAnalysis to something recent in the old servicing branch? |
Speculating: it might call Windows interfaces only when executed on Windows and |
This might be the key @trylek. We run The Can one command do what we need on Linux and macOS❔ Or, should we just add the appropriate |
Our main concern is the unreadability of our 2.1 builds due to the |
Calling the compiler twice is the right thing to do - in fact that's one of Crossgen1 drawbacks we fixed in Crossgen2 which is able to generate the PDB or perfmap directly alongside the compiled native executable, simplifying the overall workflow and increasing compilation throughput. My current working theory is that Crossgen1 simply looks at the targeting DLL for pinvokes (I doubt it actually needs to but it just might). So I'm guessing that in the "past when it was working" either the DiaSymReader was (perhaps erroneously) available even to Linux builds or the version of Microsoft.CodeAnalysis wasn't using pinvokes to the library at that time. |
Thanks, I think that's it for the However many other assemblies from Microsoft.AspNetCore.App and Microsoft.AspNetCore.All do still land in Microsoft.AspNetCore.App.Runtime packages and are |
Probably yes; however, my knowledge of the old |
The added attributes on the |
@dougbu Have you tried |
- version flows into `$(MicrosoftNETCoreApp21PackageVersion)` automatically - cherry-picked from #30729 internal branch
- skip `SharedFxTests.BaselineTest(...)` between rebranding and baseline updates - previous runtime is not available in this window - reduce retries in `RetryHelper` - download will usually fail repeatedly if it fails even once - previous maximum duration (105 min.) was greater than the job timeout - avoid NETSDK1023 warnings - eng/targets/CSharp.Common.props adds packages removed from project - cherry-picked from #30729 internal branch nit: remove unused `GetTotalRetriesUsed()` method
- run `locale-gen` to avoid "setlocale: LC_ALL: cannot change locale" and similar - needs `locales` package - move to still-supported Ubuntu 18.04 image - image needs slightly-newer `libicu60` package - rename Dockerfile to match - cherry-picked from #30729 internal branch for Ubuntu image update nit: set locale-related environment variables to match Python default
- avoid MSB4011 warnings (about repeated Directory.Build.targets imports) - place `$DOTNET_HOME` beside repo root, not under it - cherry-picked from #30729 internal branch for image update nits: - use current Docker image for 2.1 alpine - set `$LANGUAGE` with other locale-related environment variables
- run `locale-gen` to avoid "setlocale: LC_ALL: cannot change locale" and similar - needs `locales` package - move to still-supported Ubuntu 18.04 image - image needs slightly-newer `libicu60` package - rename Dockerfile to match - cherry-picked from #30729 internal branch for Ubuntu image update nit: set locale-related environment variables to match Python default
- avoid MSB4011 warnings (about repeated Directory.Build.targets imports) - place `$DOTNET_HOME` beside repo root, not under it - cherry-picked from #30729 internal branch for image update nits: - use current Docker image for 2.1 alpine - set `$LANGUAGE` with other locale-related environment variables
- skip `SharedFxTests.BaselineTest(...)` between rebranding and baseline updates - previous runtime is not available in this window - reduce retries in `RetryHelper` - download will usually fail repeatedly if it fails even once - previous maximum duration (105 min.) was greater than the job timeout - avoid NETSDK1023 warnings - eng/targets/CSharp.Common.props adds packages removed from project - create archives based on one that actually exists between rebranding and baseline updates - mostly cherry-picked from #30729 internal branch nit: remove unused `GetTotalRetriesUsed()` method
- run `locale-gen` to avoid "setlocale: LC_ALL: cannot change locale" and similar - needs `locales` package - move to still-supported Ubuntu 18.04 image - image needs slightly-newer `libicu60`, `clang` and `lldb` packages - rename Dockerfile to match - cherry-picked from #30729 internal branch for Ubuntu image update nit: set locale-related environment variables to match Python default
- avoid MSB4011 warnings (about repeated Directory.Build.targets imports) - place `$DOTNET_HOME` beside repo root, not under it - cherry-picked from #30729 internal branch for image update nits: - use current Docker image for 2.1 alpine - set `$LANGUAGE` with other locale-related environment variables
* Update branding to 2.1.28 * Bump `$(MicrosoftNETCoreAppPackageVersion)` - version flows into `$(MicrosoftNETCoreApp21PackageVersion)` automatically - cherry-picked from #30729 internal branch * Get tests and archives working after rebranding - skip `SharedFxTests.BaselineTest(...)` between rebranding and baseline updates - previous runtime is not available in this window - reduce retries in `RetryHelper` - download will usually fail repeatedly if it fails even once - previous maximum duration (105 min.) was greater than the job timeout - avoid NETSDK1023 warnings - eng/targets/CSharp.Common.props adds packages removed from project - create archives based on one that actually exists between rebranding and baseline updates - mostly cherry-picked from #30729 internal branch nit: remove unused `GetTotalRetriesUsed()` method * Use Ubuntu 18.04 build agents - set locale consistently on all platforms - default locale on newer agents is unloved `C.UTF-8` * Stop using Ubuntu 14.04 - run `locale-gen` to avoid "setlocale: LC_ALL: cannot change locale" and similar - needs `locales` package - move to still-supported Ubuntu 18.04 image - image needs slightly-newer `libicu60`, `clang` and `lldb` packages - rename Dockerfile to match - cherry-picked from #30729 internal branch for Ubuntu image update nit: set locale-related environment variables to match Python default * Avoid KoreBuild issues when using dockerbuild.sh - avoid MSB4011 warnings (about repeated Directory.Build.targets imports) - place `$DOTNET_HOME` beside repo root, not under it - cherry-picked from #30729 internal branch for image update nits: - use current Docker image for 2.1 alpine - set `$LANGUAGE` with other locale-related environment variables * Remove obsolete Groovy code * Pick up latest EF sources - minor change shouldn't affect submodule build but best to keep up * Improve the CDN tests - apply efe9b95 to similar template test - add debug output to CDN tests - back-port test reliability fix - based on @javiercn's 3368ea6 / #13646 fix - avoid `TimeSpan` arithmetic and did not match refactoring done in 'release/3.1' - do not have recently-added `TimeSpan` operators in 2.1 - skip `CdnScriptTaghelperTests` on .NET Framework - failures appear specific to that platform - leave src/Templating/test/Templates.Test/CdnScriptTagTests.cs unchanged - test project does not target .NET Framework
I'll reopen this after restoring the fixes lost here |
$(BuildNumber)
property in all build steps$(BuildScriptArgs)
name to avoid circular referencenits:
$(BuildNumberArg)
in$(SharedFxArgs)
Also includes overall build fix, copied from #30164