Skip to content

Always install node & build node components in CI #53154

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Jan 9, 2024
Merged

Always install node & build node components in CI #53154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
23f2848
Always install node in CI
wtgodbe Jan 4, 2024
4a78ef4
Update ci.yml
javiercn Jan 5, 2024
c1dcff0
Update to Alpine 3.17
javiercn Jan 5, 2024
1fdc3e2
Update .azure/pipelines/ci.yml
javiercn Jan 5, 2024
49e9895
Update .azure/pipelines/ci.yml
javiercn Jan 5, 2024
d635847
Update .azure/pipelines/ci.yml
javiercn Jan 5, 2024
dc11a2a
Update ci.yml
wtgodbe Jan 5, 2024
aca017a
[Blazor] Remove checked-in JS files (#53209)
javiercn Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 4 additions & 19 deletions .azure/pipelines/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,14 +362,12 @@ stages:
--arch arm64
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-MacOS_arm64.xml
$(_BuildArgs)
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
installNodeJs: false
artifacts:
- name: MacOS_arm64_Logs
path: artifacts/log/
Expand All @@ -393,14 +391,12 @@ stages:
buildArgs:
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-MacOS_x64.xml
$(_BuildArgs)
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
installNodeJs: false
artifacts:
- name: MacOS_x64_Logs
path: artifacts/log/
Expand All @@ -427,7 +423,6 @@ stages:
--arch x64
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
$(_BuildArgs)
Expand All @@ -447,7 +442,6 @@ stages:
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
displayName: Build RPM installers
installNodeJs: false
artifacts:
- name: Linux_x64_Logs
path: artifacts/log/
Expand All @@ -471,14 +465,12 @@ stages:
--arch arm
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-Linux_arm.xml
$(_BuildArgs)
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
installNodeJs: false
artifacts:
- name: Linux_arm_Logs
path: artifacts/log/
Expand All @@ -504,7 +496,6 @@ stages:
--arch arm64
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
$(_BuildArgs)
Expand All @@ -518,7 +509,6 @@ stages:
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
displayName: Build RPM installers
installNodeJs: false
artifacts:
- name: Linux_arm64_Logs
path: artifacts/log/
Expand All @@ -538,13 +528,12 @@ stages:
jobName: Linux_musl_x64_build
jobDisplayName: "Build: Linux Musl x64"
agentOs: Linux
container: mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.14-WithNode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This thing is pinning the nodejs version and it's using a very old version.

I've switched to 3.17 and let the Install Nodejs task do the right thing.

Is there a reason why we don't have an image that it is based on alpine:latest (3.19) so that we can alternatively install nodejs in the image? (In case we need it). For example, latest comes with nodejs 20.x, which is ideally what we want.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We just haven't set one up yet. The ownership of creating docker images has always been somewhat obtuse to me, but we're allowed to add images to https://github.com/dotnet/dotnet-buildtools-prereqs-docker if we want. I'd probably just add a new node image to the https://github.com/dotnet/dotnet-buildtools-prereqs-docker/tree/main/src/alpine/3.18 set. What errors were you seeing with the other images you tried out in this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://learn.microsoft.com/en-us/azure/devops/pipelines/process/container-phases?view=azure-devops#linux-based-containers

See https://learn.microsoft.com/en-us/azure/devops/pipelines/process/container-phases?view=azure-devops#bring-your-own-nodejs

We should start with something like https://learn.microsoft.com/en-us/azure/devops/pipelines/process/container-phases?view=azure-devops#full-example-of-a-dockerfile but obviously using a modern alpine image 3.19 contains node and can be installed via apk update && apk add nodejs npm which brings in node 20.10.

Ideally what we want is make sure that the node we use in the build is the one that we install, as that "frees us" from having to install it through a separate mechanism.

container: mcr.microsoft.com/dotnet-buildtools/prereqs:alpine-3.19-WithNode-20240105204957-5245c86
buildArgs:
--arch x64
--os-name linux-musl
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-Linux_musl_x64.xml
Expand Down Expand Up @@ -573,20 +562,18 @@ stages:
jobDisplayName: "Build: Linux Musl ARM"
agentOs: Linux
useHostedUbuntu: false
container: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-cross-arm-alpine
container: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-cross-arm-alpine
buildArgs:
--arch arm
--os-name linux-musl
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-Linux_musl_arm.xml
$(_BuildArgs)
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
installNodeJs: false
artifacts:
- name: Linux_musl_arm_Logs
path: artifacts/log/
Expand All @@ -607,20 +594,18 @@ stages:
jobDisplayName: "Build: Linux Musl ARM64"
agentOs: Linux
useHostedUbuntu: false
container: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-18.04-cross-arm64-alpine
container: mcr.microsoft.com/dotnet-buildtools/prereqs:ubuntu-22.04-cross-arm64-alpine
buildArgs:
--arch arm64
--os-name linux-musl
--pack
--all
--no-build-nodejs
--no-build-java
-p:OnlyPackPlatformSpecificPackages=true
-p:AssetManifestFileName=aspnetcore-Linux_musl_arm64.xml
$(_BuildArgs)
$(_PublishArgs)
$(_InternalRuntimeDownloadArgs)
installNodeJs: false
artifacts:
- name: Linux_musl_arm64_Logs
path: artifacts/log/
Expand Down Expand Up @@ -736,7 +721,7 @@ stages:
parameters:
platform:
name: 'Managed'
container: 'mcr.microsoft.com/dotnet-buildtools/prereqs:centos-stream8'
container: 'mcr.microsoft.com/dotnet-buildtools/prereqs:centos-stream8-20240104210103-4893224'
buildScript: './eng/build.sh $(_PublishArgs) --no-build-repo-tasks $(_InternalRuntimeDownloadArgs)'
skipPublishValidation: true
jobProperties:
Expand Down
2 changes: 1 addition & 1 deletion eng/Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@
<ProjectToBuild Condition=" $(BuildNative) " Include="@(NativeProjects)" Exclude="@(ProjectToExclude)" />
<ProjectToExclude Condition=" !$(BuildNative) " Include="@(NativeProjects)" />

<NodeJsProjects Condition="'$(TargetArchitecture)' == 'x64'"
<NodeJsProjects
Include="$(RepoRoot)eng\Npm.Workspace.nodeproj;
$(RepoRoot)eng\Npm.Workspace.FunctionalTests.nodeproj;"
AdditionalProperties="BuildInParallel=false"
Expand Down
1 change: 1 addition & 0 deletions eng/Npm.Workspace.nodeproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<Import Project="$(MSBuildThisFileDirectory)..\Directory.Build.props" />

<PropertyGroup>
<ExcludeFromSourceBuild>false</ExcludeFromSourceBuild>
<IsTestProject>true</IsTestProject>
<IsUnitTestProject>true</IsUnitTestProject>
<TestDependsOnAspNetPackages>false</TestDependsOnAspNetPackages>
Expand Down
31 changes: 31 additions & 0 deletions eng/SourceBuild.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,35 @@
</ItemGroup>
</Target>

<Target Name="RestoreNpmPackages"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the prereqs for this? I am wondering if changes are going to be needed to the containers used in SB CI. This should be flushed out to prevent blocking the dependency flow into installer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The image needs node 20 (the LTS) version to be part of the image.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only one of the container images used in SB CI has node - ubuntu 22.04 and it is extremely old. The list is specified here - https://github.com/dotnet/installer/blob/main/eng/pipelines/templates/stages/vmr-build.yml#L19 Can someone from aspnet add node prior to this flowing into installer?

Regarding the version - has there been any discussion with @dotnet/distro-maintainers to ensure this is the minimum version supported across the distros that will source-build .NET 9.0? @leecow this is something to raise in the next source-build partner sync.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @MichaelSimons ! Since this becomes a build-time dependency with minimum version, I agree on calling this out with distros and partners. After all, this impacts whether/where we can build .NET 9.

Hopefully, it shouldn't be an issue this time around.

https://github.com/nodejs/release#release-schedule is a good resource for Node.js versioning. The last version before 20 (18) goes EOL early 2025 - so just a few months after .NET 9 GA. I don't think using that is really an option for .NET 9, unless a rebase is on the table during a servicing release just a few months post GA. Even Node.js 20 goes EOL in mid-2026, (almost) aligning with the expected .NET 9 EOL.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelSimons we've already added Node to centos-stream8, as well as new alpine image - alpine-3.19-WithNode. Does every image in that list need to have node added?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to test on each of these distros. We can update to a newer version per what .NET 9 will support when it releases (e.g. updating to the new Alpine 3.19 image)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha - so to make sure I understand:

  • alpine-3.17: can be updated to alpine-3.19-WithNode
  • centos-stream8: already has Node 20
  • centos-stream9, fedora-39, ubuntu-22.04. ubuntu-22.04-arm64 : need to add Node 20

Does that sound right? If so I can do that today.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that sound right? If so I can do that today.

Yes that sounds correct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dotnet/dotnet-buildtools-prereqs-docker#949

BeforeTargets="RunInnerSourceBuildCommand">

<Message Text="Checking node version..." Importance="high" />
<Exec
Command="node --version"
WorkingDirectory="$(InnerSourceBuildRepoRoot)" />

<Message Text="Checking npm version..." Importance="high" />
<Exec
Command="npm --version"
WorkingDirectory="$(InnerSourceBuildRepoRoot)" />

<Exec
Command="npm ci"
WorkingDirectory="$(InnerSourceBuildRepoRoot)" />

</Target>

<Target Name="BuildNpmFiles"
DependsOnTargets="RestoreNpmPackages"
BeforeTargets="RunInnerSourceBuildCommand">

<Message Text="Building Node JS files..." Importance="high" />

<Exec
Command="npm run build"
WorkingDirectory="$(InnerSourceBuildRepoRoot)" />

</Target>

</Project>
2 changes: 1 addition & 1 deletion eng/common/templates/jobs/source-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ parameters:
# This is the default platform provided by Arcade, intended for use by a managed-only repo.
defaultManagedPlatform:
name: 'Managed'
container: 'mcr.microsoft.com/dotnet-buildtools/prereqs:centos-stream8'
container: 'mcr.microsoft.com/dotnet-buildtools/prereqs:centos-stream8-20240104210103-4893224'

# Defines the platforms on which to run build jobs. One job is created for each platform, and the
# object in this array is sent to the job template as 'platform'. If no platforms are specified,
Expand Down
17 changes: 6 additions & 11 deletions src/Components/Endpoints/src/Microsoft.AspNetCore.Components.Endpoints.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,15 @@
<Reference Include="Microsoft.Extensions.FileProviders.Embedded" />
</ItemGroup>

<PropertyGroup>
<BlazorWebJSFilename>blazor.web.js</BlazorWebJSFilename>
<BlazorWebJSFile Condition=" '$(Configuration)' == 'Debug' ">..\..\Web.JS\dist\Debug\$(BlazorWebJSFilename)</BlazorWebJSFile>
<BlazorWebJSFile Condition=" '$(Configuration)' != 'Debug' ">..\..\Web.JS\dist\Release\$(BlazorWebJSFilename)</BlazorWebJSFile>
</PropertyGroup>

<Target Name="_CheckBlazorWebJSPath" AfterTargets="ResolveProjectReferences" Condition=" !EXISTS('$(BlazorWebJSFile)') ">
<Warning Text="'$(BlazorWebJSFile)' does not exist. Falling back to checked-in copy." />
<Target Name="_AddEmbeddedBlazorWebJS" BeforeTargets="_CalculateEmbeddedFilesManifestInputs">
<PropertyGroup>
<BlazorWebJSFile>..\..\Web.JS\dist\Release\$(BlazorWebJSFilename)</BlazorWebJSFile>
<BlazorWebJSFilename>blazor.web.js</BlazorWebJSFilename>
<BlazorWebJSFile Condition=" '$(Configuration)' == 'Debug' ">$(MSBuildThisFileDirectory)..\..\Web.JS\dist\Debug\$(BlazorWebJSFilename)</BlazorWebJSFile>
<BlazorWebJSFile Condition=" '$(Configuration)' != 'Debug' ">$(MSBuildThisFileDirectory)..\..\Web.JS\dist\Release\$(BlazorWebJSFilename)</BlazorWebJSFile>
</PropertyGroup>
</Target>

<Target Name="_AddEmbeddedBlazorWebJS" AfterTargets="_CheckBlazorWebJSPath">
<Warning Condition="!Exists('$(BlazorWebJSFile)')" Text="'$(BlazorWebJSFile)' does not exist. Ensure the JS assets have been build by running 'npm run build' from the repository root." />

<ItemGroup>
<EmbeddedResource Include="$(BlazorWebJSFile)" LogicalName="_framework/$(BlazorWebJSFilename)" />
<EmbeddedResource Include="$(BlazorWebJSFile).map" LogicalName="_framework/$(BlazorWebJSFilename).map" Condition="Exists('$(BlazorWebJSFile).map')" />
Expand Down
17 changes: 6 additions & 11 deletions src/Components/Server/src/Microsoft.AspNetCore.Components.Server.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,20 +98,15 @@
<Compile Include="$(RepoRoot)src\Shared\Components\ProtectedPrerenderComponentApplicationStore.cs" />
</ItemGroup>

<PropertyGroup>
<BlazorServerJSFilename>blazor.server.js</BlazorServerJSFilename>
<BlazorServerJSFile Condition=" '$(Configuration)' == 'Debug' ">..\..\Web.JS\dist\Debug\$(BlazorServerJSFilename)</BlazorServerJSFile>
<BlazorServerJSFile Condition=" '$(Configuration)' != 'Debug' ">..\..\Web.JS\dist\Release\$(BlazorServerJSFilename)</BlazorServerJSFile>
</PropertyGroup>

<Target Name="_CheckBlazorServerJSPath" AfterTargets="ResolveProjectReferences" Condition=" !EXISTS('$(BlazorServerJSFile)') ">
<Warning Text="'$(BlazorServerJSFile)' does not exist. Falling back to checked-in copy." />
<Target Name="_AddEmbeddedBlazor" BeforeTargets="_CalculateEmbeddedFilesManifestInputs">
<PropertyGroup>
<BlazorServerJSFile>..\..\Web.JS\dist\Release\$(BlazorServerJSFilename)</BlazorServerJSFile>
<BlazorServerJSFilename>blazor.server.js</BlazorServerJSFilename>
<BlazorServerJSFile Condition=" '$(Configuration)' == 'Debug' ">$(MSBuildThisFileDirectory)..\..\Web.JS\dist\Debug\$(BlazorServerJSFilename)</BlazorServerJSFile>
<BlazorServerJSFile Condition=" '$(Configuration)' != 'Debug' ">$(MSBuildThisFileDirectory)..\..\Web.JS\dist\Release\$(BlazorServerJSFilename)</BlazorServerJSFile>
</PropertyGroup>
</Target>

<Target Name="_AddEmbeddedBlazor" AfterTargets="_CheckBlazorServerJSPath">
<Warning Condition="!Exists('$(BlazorServerJSFile)')" Text="'$(BlazorServerJSFile)' does not exist. Ensure the JS assets have been build by running 'npm run build' from the repository root." />

<ItemGroup>
<EmbeddedResource Include="$(BlazorServerJSFile)" LogicalName="_framework/$(BlazorServerJSFilename)" />
<EmbeddedResource Include="$(BlazorServerJSFile).map" LogicalName="_framework/$(BlazorServerJSFilename).map" Condition="Exists('$(BlazorServerJSFile).map')" />
Expand Down
5 changes: 2 additions & 3 deletions src/Components/Web.JS/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
node_modules/
dist/Debug/
dist/Release/blazor.webassembly.js
dist/Release/blazor.webview.js
dist/
dist/
1 change: 0 additions & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/Components/Web.JS/dist/Release/blazor.web.js
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion src/Components/Web.JS/dist/Release/blazor.webview.js
View file
Open in desktop

This file was deleted.

16 changes: 6 additions & 10 deletions src/Components/WebView/WebView/src/Microsoft.AspNetCore.Components.WebView.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,16 @@

<UsingTask AssemblyFile="$(_FileProviderTaskAssembly)" TaskName="Microsoft.Extensions.FileProviders.Embedded.Manifest.Task.GenerateEmbeddedResourcesManifest" />

<PropertyGroup>
<BlazorWebViewJSFilename>blazor.webview.js</BlazorWebViewJSFilename>
<BlazorWebViewJSFile Condition=" '$(Configuration)' == 'Debug' ">..\..\..\Web.JS\dist\Debug\$(BlazorWebViewJSFilename)</BlazorWebViewJSFile>
<BlazorWebViewJSFile Condition=" '$(Configuration)' != 'Debug' ">..\..\..\Web.JS\dist\Release\$(BlazorWebViewJSFilename)</BlazorWebViewJSFile>
</PropertyGroup>
<Target Name="_AddEmbeddedBlazorWebView" BeforeTargets="_CalculateEmbeddedFilesManifestInputs">

<Target Name="_CheckBlazorWebViewJSPath" AfterTargets="ResolveProjectReferences" Condition=" !EXISTS('$(BlazorWebViewJSFile)') ">
<Warning Text="'$(BlazorWebViewJSFile)' does not exist. Falling back to checked-in copy." />
<PropertyGroup>
<BlazorWebViewJSFile>..\..\..\Web.JS\dist\Release\$(BlazorWebViewJSFilename)</BlazorWebViewJSFile>
<BlazorWebViewJSFilename>blazor.webview.js</BlazorWebViewJSFilename>
<BlazorWebViewJSFile Condition=" '$(Configuration)' == 'Debug' ">$(MSBuildThisFileDirectory)..\..\..\Web.JS\dist\Debug\$(BlazorWebViewJSFilename)</BlazorWebViewJSFile>
<BlazorWebViewJSFile Condition=" '$(Configuration)' != 'Debug' ">$(MSBuildThisFileDirectory)..\..\..\Web.JS\dist\Release\$(BlazorWebViewJSFilename)</BlazorWebViewJSFile>
</PropertyGroup>
</Target>

<Target Name="_AddEmbeddedBlazorWebView" BeforeTargets="_CalculateEmbeddedFilesManifestInputs" DependsOnTargets="_CheckBlazorWebViewJSPath">
<Warning Condition="!Exists('$(BlazorWebViewJSFile)')" Text="'$(BlazorWebViewJSFile)' does not exist. Ensure the JS assets have been build by running 'npm run build' from the repository root." />

<ItemGroup>
<EmbeddedResource Include="blazor.modules.json" LogicalName="_framework/blazor.modules.json" />
<EmbeddedResource Include="$(BlazorWebViewJSFile)" LogicalName="_framework/$(BlazorWebViewJSFilename)" />
Expand Down
4 changes: 3 additions & 1 deletion src/SignalR/clients/ts/FunctionalTests/scripts/karma.local.conf.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
const os = require('os');

try {
// Karma configuration for a local run (the default)
const createKarmaConfig = require("./karma.base.conf");
Expand Down Expand Up @@ -45,7 +47,7 @@ try {
// We use the launchers themselves to figure out if the browser exists. It's a bit sneaky, but it works.
tryAddBrowser("ChromeHeadlessNoSandbox", ChromeHeadlessBrowser.prototype);
tryAddBrowser("ChromiumHeadlessIgnoreCert", ChromiumHeadlessBrowser.prototype);
if (!tryAddBrowser("FirefoxHeadless", FirefoxHeadlessBrowser.prototype)) {
if (os.platform() !== 'darwin' && !tryAddBrowser("FirefoxHeadless", FirefoxHeadlessBrowser.prototype)) {
tryAddBrowser("FirefoxDeveloperHeadless", FirefoxDeveloperHeadlessBrowser.prototype);
}

Expand Down