Skip to content

Conversation

@NikolaMilosavljevic
Copy link
Contributor

Bug

Fixes: NuGet/Home#12598

Regression? No

Description

Enables prebuilt detection, that causes build failure for any new prebuilts not in baseline.

Since it's not possible to flow dependencies, i.e. SBRP intermediate package, all prebuilts have to be listed in prebuilt baseline: https://github.com/NuGet/NuGet.Client/blob/dev/eng/SourceBuildPrebuiltBaseline.xml

All .NET packages are excluded with * version, to avoid unnecessary changes in the future - i.e.:
<UsagePattern IdentityGlob="System.Buffers/*" />

All instances of other packages are excluded with specific version, to ensure that version update triggers proper prebuilt update process, i.e.:
<UsagePattern IdentityGlob="Newtonsoft.Json/13.0.1" />

@NikolaMilosavljevic NikolaMilosavljevic requested a review from a team as a code owner May 25, 2023 16:32
@ghost ghost added the Community PRs created by someone not in the NuGet team label May 25, 2023
@NikolaMilosavljevic
Copy link
Contributor Author

If you get an error in source-build leg pointing at new prebuilt package being detected,
resolve the prebuilt issue before merging your PR.
If that is not possible, do the following:
- contact source-build team
Copy link
Contributor

Choose a reason for hiding this comment

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

I would include the contact info - use the dotnet/source-build-internal github team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in eaaaebc

resolve the prebuilt issue before merging your PR.
If that is not possible, do the following:
- contact source-build team
- add required UsagePattern entry as provided by source-build team
Copy link
Contributor

Choose a reason for hiding this comment

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

I tend to think we should exclude anything besides resolving the prebuilt and contacting the source-build team. I fear that suggesting creating UsagePatterns is going to lead to the problem we are trying to prevent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense - we want source-build team to provide guidance on a case-by-case basis. Will update - thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in eaaaebc


<MSbuild Projects="$(ArcadeDir)/SourceBuild/AfterSourceBuild.proj"
Properties="_NETCORE_ENGINEERING_TELEMETRY=AfterSourceBuild;ArcadeBuildFromSource=true;CurrentRepoSourceBuildArtifactsPackagesDir=$(ProjectRoot)artifacts/nupkgs/"
Properties="_NETCORE_ENGINEERING_TELEMETRY=AfterSourceBuild;ArcadeBuildFromSource=true;CurrentRepoSourceBuildArtifactsPackagesDir=$(ProjectRoot)artifacts/nupkgs/;FailOnPrebuiltBaselineError=true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this negatively affect the product source-build? When doing a full source-build, we don't want this repo to stop the build if it has prebuilts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right - it will affect full source-build. I think we should be able to make this change specific to repo source-build. Will update - thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in eaaaebc


ReadGlobalVersion Microsoft.DotNet.Arcade.Sdk
export ARCADE_VERSION=$_ReadGlobalVersion
export NUGET_PACKAGES=$scriptroot/../../artifacts/source-build/self/package-cache/
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used/exist in the full repo source-build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is set in full source-build: https://github.com/dotnet/dotnet/blob/8528c489bcbaeff232b906a3c784016f2de74fe2/build.sh#L248

I think I need to condition this for repo build only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved in eaaaebc

NikolaMilosavljevic and others added 2 commits May 26, 2023 08:44
@NikolaMilosavljevic NikolaMilosavljevic changed the title Prebuilts Enable source-build pre-built detection May 27, 2023
@NikolaMilosavljevic
Copy link
Contributor Author

NikolaMilosavljevic commented May 30, 2023

@kartheekp-ms @zivkan can this be approved for CI? This change is specific to .NET source-build.

@jeffkl jeffkl self-assigned this May 30, 2023
@jeffkl
Copy link
Contributor

jeffkl commented May 30, 2023

@kartheekp-ms @zivkan can this be approved for CI? This change is specific to .NET source-build.

Done!

@NikolaMilosavljevic
Copy link
Contributor Author

@kartheekp-ms @zivkan can this be approved for CI? This change is specific to .NET source-build.

Done!

There are some test failures - but since those are on Windows and this PR is only for Linux and source-build, they are unrelated. I see the same failures in other recent PRs.

@jeffkl
Copy link
Contributor

jeffkl commented May 30, 2023

There are some test failures - but since those are on Windows and this PR is only for Linux and source-build, they are unrelated. I see the same failures in other recent PRs.

Yes those test failures are currently known and we'll ignore them. I'll just give the team one more day to review and I'll merge tomorrow. Thanks for the contribution, glad this will prevent us from breaking source build going forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community PRs created by someone not in the NuGet team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enable source-build pre-built detection

4 participants