-
Notifications
You must be signed in to change notification settings - Fork 353
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
Publish installers to dotnetcli / dotnetclichecksums for the dev chan… #3697
Conversation
src/Microsoft.DotNet.Arcade.Sdk/tools/SdkTasks/SetupTargetFeeds.proj
Outdated
Show resolved
Hide resolved
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.
Fantastic
src/Microsoft.DotNet.Arcade.Sdk/tools/SdkTasks/SetupTargetFeeds.proj
Outdated
Show resolved
Hide resolved
New test build with all the latest changes and publishing to a test container in the storage accounts: https://dev.azure.com/dnceng/internal/_build/results?buildId=315910 Relevant logs:
Container in dotnetclichecksums: @mmitche and @JohnTortugo this is ready for review again. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -123,6 +136,16 @@ | |||
TargetURL="$(TargetStaticFeed)" | |||
Type="AzureStorageFeed" | |||
Token="$(AzureStorageTargetFeedPAT)" /> | |||
<TargetFeedConfig Condition="'$(PublishInstallersAndChecksums)' == 'true'" | |||
Include="Deb;Rpm;Node;BinaryLayout;Installer;Maven;VSIX" |
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.
This binary layout thing (.zip files, right?) is also an installer?
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.
Yeah, zips, msis, etc. should all be on this layout. Does this handle that @riarenas
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.
Zip is handled by BinaryLayout, MSI is handled by Installer category
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.
LGTM
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.
Approved except for the question about the zips
@@ -43,6 +48,14 @@ | |||
Condition="('$(AzureStorageAccountName)' == '' or '$(AzureStorageAccountKey)' == '') and '$(IsStableBuild)' == 'true'" | |||
Text="Parameters 'AzureStorageAccountName/Key' are empty." /> | |||
|
|||
<Error |
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.
Does this handle stabilized builds? Publishing to a location that will not overwrite existing binaries?
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.
For stabilized binaries we already create a new container / feed on every build and publish everything there.
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.
Okay, so the installers make it there too.
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.
Yep
Arcade does not publish installers or checksums, so disable this.
Changes to enable publishing installers to dotnetcli and checksums to dotnetclichecksums for #3607
I have only made the changes for the public channels.
I'm still testing to see if we can use proxies, or if the private version of this should wait until #3684
I'll provide a newer test build when I hack my stuff around the dotnet-install stuff, but wanted to see if this is the path we want to use.
This also cannot be merged until we have a new arcade SDK that includes the changes in #3696
I'm also thinking of adding a parameter to opt out of this publishing, for repos that already implemented something else for this.