-
Notifications
You must be signed in to change notification settings - Fork 42
Enable publishing in VMR #275
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
|
Will update this to match similar changes in symreader and fsharp. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this PR needed? Isn't that the default? Which packages aren't picked up?
|
Yeah, agreed that these should already be the defaults |
|
Interestingly publishing looks at artifacts/packages/shipping and nonshipping while signing looks at artifacts/packages/*: https://github.com/dotnet/arcade/blob/0f35699621fdb3a08fa2840c9d6d08769438e243/src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props#L14 I think this should be aligned eventually, maybe with the VMR signing work. |
|
I think there is some subtlety here. The release package generation (generates release and prerelease versions of the shipping packages) sticks everything under packages\Release and packages\PreRelease. All of it needs to be signed, but only the shipping and nonshipping dirs are reported to BAR for dependency flow. |
|
Yeah - these packages are not picked by Arcade infra as it's more restrictive in folder set for nupkg collection. We need this change. |
|
@NikolaMilosavljevic which packages are currently not picked up by publishing? Please give an example. |
With changes in this PR, all SBE packages are picked up. Without the changes, none of the packages are picked up. Just 2 examples: This is due to different sub-folder naming in packages folder. Default arcade infra collects packages from |
Interesting. Why is that so? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving to get things flowing but we should follow-up on why SBE puts the artifacts under packages/Release instead of the expected directories.
| <ItemGroup> | ||
| <ItemsToPushToBlobFeed Include="$(ArtifactsPackagesDir)**\*.nupkg" | ||
| IsShipping="true" | ||
| UploadPathSegment="Runtime" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NikolaMilosavljevic I don't think this line is necessary and should probably be deleted. Please also add a tracking issue for investigating why packages are not in the expected subfolders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created an issue to fix this and remove custom publishing infra: dotnet/source-build#4235
Contributes to dotnet/source-build#4101
Enables publishing in VMR.
This repo creates packages in locations that aren't collected by common publishing infra in arcade. We need to collect them here.