Skip to content

Sbom for VS insertion #7826

Merged
Forgind merged 3 commits intodotnet:mainfrom
epananth:sbom-fix
Jul 17, 2022
Merged

Sbom for VS insertion #7826
Forgind merged 3 commits intodotnet:mainfrom
epananth:sbom-fix

Conversation

@epananth
Copy link
Copy Markdown
Member

Fixes #

Context

There was a build error earlier, I fixed it. I have added the appropriate test to this.

Changes Made

Testing

https://dev.azure.com/devdiv/DevDiv/_build/results?buildId=6416151&view=results

Notes

@epananth epananth requested a review from rainersigwald July 14, 2022 19:28
"dotnet": "6.0.200",
"runtimes": {
"dotnet": [
"3.1.0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is fine, but please consider configuring the tool for rollforward/major so it doesn't impose this requirement on other repos.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I created an issue for the manifest tool folks to add it https://github.com/microsoft/dropvalidator/issues/454, but I did not hear back from them, so I created a workaround

Copy link
Copy Markdown
Contributor

@jrdodds jrdodds Jul 17, 2022

Choose a reason for hiding this comment

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

This change has broken the build on macOS with Apple Silicon (arm64).

dotnet-install: The resource at legacy link 'https://dotnetbuilds.azureedge.net/public/Runtime/3.1.0/dotnet-osx-arm64.3.1.0.tar.gz' is not available.
dotnet_install: Error: Could not find `.NET Core Runtime` with version = 3.1.0
dotnet_install: Error: Refer to: https://aka.ms/dotnet-os-lifecycle for information on .NET Core support

Opened bug #7834

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@benvillalobos, is there some way to work around #7834 without either reverting this or making the arcade change rainersigwald suggested?

The arcade change is my favorite option. If there's no workaround for jrdodds, I'd favor reverting this; having to ignore the sbom thing every time is annoying, but at least we can get around it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just for clarity, since I recognize that you can work around this by locally reverting the global.json change, I was wondering if there's something we can do that would let our official builds pass and create insertions PRs with SBoM without requiring a change on an arm64 machine before building.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there some way to work around #7834 without either reverting this or making the arcade change rainersigwald suggested?

Not that I'm aware of. Everything under "tools" seems to be an arcade concept.

If there's no workaround for jrdodds, I'd favor reverting this

Fully agree. We're officially supporting arm64 now, we should treat "working arm64 builds" as a requirement. It's better that we bite the bullet and continue doing what we've been doing with the sbom stuff.

I was wondering if there's something we can do that would let our official builds pass and create insertions PRs with SBoM without requiring a change on an arm64 machine before building.

That'd require a split in our main branches, unfortunately that's a no-go without some overly complicated git magic :(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#7836

I was thinking ideally of some magic line you can add to only respect that part of the global.json on x64/x86, but global.json doesn't have that level of customization, as far as I know.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I double checked the docs just to be sure, but didn't find anything either :/

@epananth epananth requested a review from Forgind July 14, 2022 19:42
@Forgind Forgind added the merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now. label Jul 14, 2022
@Forgind Forgind merged commit 659a296 into dotnet:main Jul 17, 2022
Forgind added a commit to Forgind/msbuild that referenced this pull request Jul 18, 2022
Forgind added a commit that referenced this pull request Jul 18, 2022
@jrdodds
Copy link
Copy Markdown
Contributor

jrdodds commented Jul 18, 2022

@Forgind I tested the revert and the issue is resolved (as expected but always good to confirm).

@Forgind
Copy link
Copy Markdown
Contributor

Forgind commented Jul 18, 2022

@Forgind I tested the revert and the issue is resolved (as expected but always good to confirm).

Thanks for checking!

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

Labels

merge-when-branch-open PRs that are approved, except that there is a problem that means we are not merging stuff right now.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants