dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.49k stars 10.03k forks source link

source-building .NET 7 fails when building npmprojects #46304

Open omajid opened 1 year ago

omajid commented 1 year ago

Is there an existing issue for this?

Describe the bug

(This is a follow up from https://github.com/dotnet/aspnetcore/issues/42378 which is now locked. Sorry about dropping the ball on that.)

When building ASP.NET Core, turning on source-build mode makes the build fail when it's trying to build nodejs projects

I am currently testing with aspnetcore commit 9ffffa45789328985d43cc70a378fd8570988f27, but based on the issue linked above, the bug has existed for a while now.

This works:

./eng/build.sh  --ci --configuration Release --restore --build --pack 

This fails:

./eng/build.sh  --ci --configuration Release --restore --build --pack /p:ArcadeBuildFromSource=true

The build failure looks like this:

    $ yarn run build:lint && yarn run build:esm && yarn run build:cjs && yarn run build:browser && yarn run build:uglify                                      
    $ node ../common/node_modules/eslint/bin/eslint ./src --ext .ts --resolve-plugins-relative-to ../common                                                   
    $ node ../common/node_modules/typescript/bin/tsc --project ./tsconfig.json --module es2015 --outDir ./dist/esm -d                                             $ node ../common/node_modules/typescript/bin/tsc --project ./tsconfig.json --module commonjs --outDir ./dist/cjs                                          
    $ node ../common/node_modules/webpack-cli/bin/cli.js                                                                                                                                                                                                                                                                    
    Duplicate Sources / Packages - No duplicates found. 🚀                                                                                                                                                                                                                                                                  
    asset signalr-protocol-msgpack.js 80.6 KiB [emitted] (name: signalr-protocol-msgpack) 1 related asset                                                         asset signalr-protocol-msgpack.min.js 28.6 KiB [emitted] [minimized] (name: signalr-protocol-msgpack.min) 1 related asset                                 
    orphan modules 89.3 KiB [orphan] 20 modules                                                                                                               
    runtime modules 1.31 KiB 6 modules                                                                                                                            built modules 76.9 KiB [built]                                                                                                                            
      ./src/browser-index.ts + 15 modules 76.9 KiB [built] [code generated]                                                                                   
        ModuleConcatenation bailout: Cannot concat with external "signalR": umd externals can't be concatenated                                               
      external "signalR" 42 bytes [built] [code generated]                                                                                                    
        ModuleConcatenation bailout: umd externals can't be concatenated                                                                                      
    webpack 5.75.0 compiled successfully in 2071 ms                                                                                                           
    $ node ../common/node_modules/terser/bin/terser -m -c --ecma 2019 --module --source-map "url='signalr-protocol-msgpack.min.js.map',content='./dist/browser/signalr-protocol-msgpack.js.map'" --comments -o ./dist/browser/signalr-protocol-msgpack.min.js ./dist/browser/signalr-protocol-msgpack.js                    
    Done in 9.65s.                                                                                                                                            
    yarn version v1.22.10                                                                                                                                     
    info Current version: 5.0.0-dev                                                                                                                           
    info Visit https://yarnpkg.com/en/docs/cli/version for documentation about this command.                                                                  
    error Invalid version supplied.                                                                                                                           
  /aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets(108,5): error MSB6006: "yarn" exited with code 1. [/aspnetcore/artifacts/source-build/self/src/src/SignalR/clients/ts/signalr-protocol-msgpack/signalr-protocol-msgpack.npmproj]             
  ##vso[task.logissue type=error;sourcepath=/aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets;linenumber=108;columnnumber=5;code=MSB6006;](NETCORE_ENGINEERING_TELEMETRY=Build) "yarn" exited with code 1.                                                                
    yarn version v1.22.10                                                                                                                                     
    info Current version: 5.0.0-dev                                                                                                                           
    error Invalid version supplied.                                                                                                                           
    info Visit https://yarnpkg.com/en/docs/cli/version for documentation about this command.                                                                  
  /aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets(108,5): error MSB6006: "yarn" exited with code 1. [/aspnetcore/artifacts/source-build/self/src/src/SignalR/clients/ts/signalr/signalr.npmproj]                                               
  ##vso[task.logissue type=error;sourcepath=/aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets;linenumber=108;columnnumber=5;code=MSB6006;](NETCORE_ENGINEERING_TELEMETRY=Build) "yarn" exited with code 1.                                                                
    yarn version v1.22.10                                                                                                                                     
    info Current version: 8.0.0-dev                                                                                                                           
    error Invalid version supplied.                                                                                                                           
    info Visit https://yarnpkg.com/en/docs/cli/version for documentation about this command.                                                                  
  /aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets(108,5): error MSB6006: "yarn" exited with code 1. [/aspnetcore/artifacts/source-build/self/src/src/JSInterop/Microsoft.JSInterop.JS/src/Microsoft.JSInterop.JS.npmproj]                      
  ##vso[task.logissue type=error;sourcepath=/aspnetcore/artifacts/source-build/self/src/eng/targets/Npm.Common.targets;linenumber=108;columnnumber=5;code=MSB6006;](NETCORE_ENGINEERING_TELEMETRY=Build) "yarn" exited with code 1.     

This didn't seem a too big of a problem back when https://github.com/dotnet/aspnetcore/issues/42378 was opened, but with https://github.com/dotnet/aspnetcore/pull/45883/, this issue makes the build break with the VMR.

From what I can tell, PackageVersion is set to empty here:

https://github.com/dotnet/aspnetcore/blob/ea683686bfac765690cb6e40f6ba7198cae26e65/eng/targets/Npm.Common.targets#L108

And that makes the yarn command fail.

I don't know why PackageVersion is empty. I know that arcade handles ${FOO}PackageVersion specially for PackageVersions.props. Could that be setting PackageVersion to empty too? Is there some way I can track what's setting PackageVersion to empty (or perhaps, not setting to the right version it when it should be set)?

$ node --version
v18.12.1
$ yarnpkg --version
1.22.19
$ npm --version
8.19.2

cc @chrisrummel @dougbu @mthalman @michaelsimons @wtgodbe

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

omajid commented 1 year ago

@crummel

wtgodbe commented 1 year ago

Getting a binlog would help - you could dotnet build the failing .npmproj with the /p:ArcadeBuildFromSource=true arg + /bl. That would show you if/when PackageVersion is being set.

omajid commented 1 year ago

Here's the full binlog: sourcebuild.binlog.tar.gz from a failing build of aspnetcore.

Building the individual project doesn't reproduce the issue :/

wtgodbe commented 1 year ago

Looks like none of our .npmproj's have PackageVersion set in that binlog, I think because they're not importing this file: https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/Version.BeforeCommonTargets.targets. Trying to figure out why.

wtgodbe commented 1 year ago

Could you also send a binlog of just building the .npmproj successfully (w/ the sourceBuild arg passed)? That could be informative on what isn't happening in the failing case.

omajid commented 1 year ago

I have a feeling I am using source-build args here incorrectly, but this is the binlog I get from:

$ cd aspnetcore
$ ./eng/build.sh  --ci --configuration Release --restore --build --pack /p:ArcadeBuildFromSource=true   # fails, as expected
$ cd artifacts/source-build/self/src/src/JSInterop/Microsoft.JSInterop.JS/src/
$ ../../../../.dotnet/dotnet msbuild /t:Pack /p:ArcadeBuildFromSource=true /bl:isolated-Microsoft.JSInterop.JS-working.binlog

isolated-Microsoft.JSInterop.JS-working.binlog.tar.gz

wtgodbe commented 1 year ago

@omajid where did you get ./eng/build.sh --ci --configuration Release --restore --build --pack /p:ArcadeBuildFromSource=true from? I'm wondering why that fails but our CI source-build leg doesn't.

omajid commented 1 year ago

It's essentially the configuration used by source-build/VMR when building aspnetcore. You can see the full command line here: https://dev.azure.com/dnceng-public/public/_build/results?buildId=154293&view=logs&j=61a52f3a-cec1-5bff-8c02-d8ba2c8bed99&t=53e46d9d-cfbd-5072-889d-e51560a71252&l=2722:

/vmr/src/aspnetcore/eng/build.sh  --ci --configuration Release --restore --build --pack  -bl /p:ArcadeBuildFromSource=true /p:CopyWipIntoInnerSourceBuildRepo=true /p:DotNetBuildOffline=true /p:CopySrcInsteadOfClone=true /p:DotNetPackageVersionPropsPath="/vmr/artifacts/obj/x64/Release/PackageVersions.aspnetcore.props" /p:AdditionalSourceBuiltNupkgCacheDir="/vmr/artifacts/obj/x64/Release/blob-feed/packages/" /p:ReferencePackageNupkgCacheDir="/vmr/prereqs/packages/reference/" /p:PreviouslySourceBuiltNupkgCacheDir="/vmr/prereqs/packages/previously-source-built/" /p:SourceBuildUseMonoRuntime= --arch x64 --no-build-repo-tasks --no-build-nodejs /p:PublishCompressedFilesPathPrefix=/vmr/artifacts/obj/x64/Release/blobs/aspnetcore/Runtime/ /p:PortableBuild=false /p:TargetRuntimeIdentifier=centos.8-x64 /p:MicrosoftNetFrameworkReferenceAssembliesVersion=1.0.0  /p:DotNetPackageVersionPropsPath=/vmr/artifacts/obj/x64/Release/PackageVersions.aspnetcore.props
omajid commented 1 year ago

Oh, and the reason the CI leg doesn't fail is probably because of --no-build-nodejs. That's present in the CI leg and the current full source-build VMR command. But it's missing in my run command.

I have removed it intentionally to test what building the VMR post https://github.com/dotnet/aspnetcore/pull/45883/ might look like.

There's some additional context at https://github.com/dotnet/source-build/discussions/3107 if you are interested.

omajid commented 1 year ago

Funny enough, this seems to build:

./eng/build.sh  --ci --configuration Release --restore --build --pack /p:ArcadeBuildFromSource=true /p:ArcadeInnerBuildFromSource=true

The /p:ArcadeInnerBuildFromSource=true is used to indicate this is the inner build. I think I am doing something wrong, though, because that's supposed to happen as part of the regular source-build anyway and should be identical (aside from different paths) to the main build.

MichaelSimons commented 1 year ago

[SB Triage] @crummel - can you please help investigate?

crummel commented 1 year ago

I think we should probably delay this investigation until after the planned yarn to npm switch if that sounds okay @omajid. @wtgodbe is there an issue for that? I wasn't able to find one.

crummel commented 1 year ago

Ah, the issue is a pretty long-standing one, I was looking for something new. it's https://github.com/dotnet/aspnetcore/issues/37398.

omajid commented 1 year ago

@crummel sure, that sounds okay. I hope this issue doesn't get auto-closed again :)