Skip to content
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 the SDK NuGet packages to the Transport Feed #1687

Merged
merged 2 commits into from Oct 26, 2017

Conversation

johnbeisner
Copy link
Contributor

Addresses Step 2: #1637

A stand-alone msbuild target to publish SDK nupkgs to the Transport feed. The 'PublishNupkgToTransportFeed' target will be invoked thru a 'VSO' style step similar to: "Publish Windows Installer Nupkg to VS Feed" and "Publish MSBuild Extensions NuPkg to VS Feed" - controlled by "PUBLISH_NUPKG_TO_TRANSPORT_FEED" Boolean.

Notes:
The value for 'TRANSPORTFEED_STORAGE_KEY' will be passed into the build via a secret parameter from VSO. The key will be stored in our Azure KeyVault.

@chcosta
@karajas
@jcagme

WorkingDirectory="$(RepositoryRootDirectory)" />
</Target>

<Target Name="PushNupkgToTransportFeed" >
Copy link
Member

Choose a reason for hiding this comment

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

This target appears to be re-implementing some of the functionality of Publish.targets from the package. Is this a temporary step (subsequent versions of the package have changed considerably)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a temporary step in the sense that the SDK repo will be folding in the CLI repo. The PR is to support step 1 completion.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be worried about temporary steps because they often become servicing branches and then updating them can be painful because they vary so much from other branches. ie, I would hate for the servicing story to be, rewrite this target to match the new target in the package. I understand that the api for this package was rough for the version you're currently using though.

<!-- Capture 'OutputPath' before Publish.csproj modifies it -->
<OutputPathTransportFeed>$(OutputPath)</OutputPathTransportFeed>
<BuildTasksFeedDllVersion>1.0.0-prerelease-01929-02</BuildTasksFeedDllVersion>
<BuildTasksFeedDll>$(NuGet_Packages)\Microsoft.DotNet.Build.Tasks.Feed\$(BuildTasksFeedDllVersion)\lib\desktop\Microsoft.DotNet.Build.Tasks.Feed.dll</BuildTasksFeedDll>
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? I suppose it is for this version of the package. When you update to the later version, you should be able to get rid of this logic and just let dotnet determine the dll and handle imports.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you elaborate on this comment - I do not think the VSO msbuild-task implementation uses 'dotnet'.

Copy link
Member

Choose a reason for hiding this comment

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

Can you send me a link to your official build for sdk so I can get a better idea for how this is executed?

<PropertyGroup>
<!-- Capture 'OutputPath' before Publish.csproj modifies it -->
<OutputPathTransportFeed>$(OutputPath)</OutputPathTransportFeed>
<BuildTasksFeedDllVersion>1.0.0-prerelease-01929-02</BuildTasksFeedDllVersion>
Copy link
Member

Choose a reason for hiding this comment

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

Personally, I'd move this into the csproj (before the import) so that you can see the package reference and version in the same place rather than having to go the targets file to see what version is used. I view the targets file more as supporting plumbing for the toggles defined in the csproj.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea...done.

]]>
</NewLineTF>
<SetNuget_PackagesTF>set NUGET_PACKAGES=$(NuGet_Packages)</SetNuget_PackagesTF>
<SetDotnet_Skip_FirstTime>set DOTNET_SKIP_FIRST_TIME_EXPERIENCE=true</SetDotnet_Skip_FirstTime>
Copy link
Member

Choose a reason for hiding this comment

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

Seems like these types of property should be defined in common.props. ie NewLineTF, SetDotnet_Skip_FirstTime, etc...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a stand-alone task - and as such, should be as self-contained as possible - this can be removed by a simple deletion of the 'Publish' directory.

@johnbeisner johnbeisner merged commit fc3e52d into dotnet:release/2.0.0 Oct 26, 2017
@johnbeisner johnbeisner deleted the SDKTransportFeed branch December 12, 2017 18:18
JL03-Yue pushed a commit that referenced this pull request Mar 19, 2024
…823.18 (#1687)

[main] Update dependencies from dotnet/roslyn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants