remove file share dependency for buildinfo.json file and download the file from artifacts during release

[kartheekp-ms]

Bug

Fixes: https://github.com/NuGet/Client.Engineering/issues/394

Regression: No

Fix

Modified CI scripts to remove file share dependency for buildinfo.json file. The following build step runs the ConfigureVstsBuild.ps1.

FolderSuffix	StepName	PhaseName	Comment
Build	Configure VSTS CI Environment	Build_and_UnitTest NonRTM	Generates buildinfo.json file

I have removed logic to support -SkipUpdateBuildNumber switch in the ConfigureVstsBuild.ps1 script as I found it redundant, and we always pass this switch to the command. Happy to bring to back based on the feedback.

NuGet.Client/build/templates/Build_and_UnitTest.yml

Line 37 in b293bd5

arguments: "-BuildCounterFile $(BuildCounterFile) -BuildInfoJsonFile $(BuildInfoJsonFile) -BuildRTM $(BuildRTM) -SkipUpdateBuildNumber"

For e.g. Build_and_UnitTest_NonRTM - https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4378624&view=logs&j=2ea8c801-c4c3-578e-0ff0-bcf4c12d9404&t=ca35e91d-75ae-5ed4-785b-bfd3ac3f2ddd&l=9

'C:\A\1\280\s\scripts\cibuild\ConfigureVstsBuild.ps1' -BuildCounterFile \\ddfiles\drops\NuGet\Drops\CI\NuGet.Client\buildcounter_official.txt -BuildInfoJsonFile \\ddfiles\drops\NuGet\Drops\CI\NuGet.Client\dev\5.9.0.7050\buildinfo.json -BuildRTM false -SkipUpdateBuildNumber

Build_and_UnitTest_RTM - https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4378624&view=logs&j=3662048e-4e2d-5ad0-bcc2-35d49e82de1c&t=f7bff3ac-56d9-56f8-3e0e-88c5057834ef&l=9

'C:\A\1\330\s\scripts\cibuild\ConfigureVstsBuild.ps1' -BuildCounterFile \\ddfiles\drops\NuGet\Drops\CI\NuGet.Client\buildcounter_official.txt -BuildInfoJsonFile \\ddfiles\drops\NuGet\Drops\CI\NuGet.Client\dev\5.9.0.7050\buildinfo.json -BuildRTM true -SkipUpdateBuildNumber

With this change buildinfo.json will not be stored on the file share and we already have a build step to publish the file to Artifcats.

NuGet.Client/build/templates/Build_and_UnitTest.yml

Lines 40 to 46 in b293bd5

	- task: PublishBuildArtifacts@1
	displayName: 'Publish buildinfo.json as an artifact'
	inputs:
	ArtifactName: 'BuildInfo'
	ArtifactType: 'Container'
	PathToPublish: '$(Build.Repository.LocalPath)\artifacts\buildinfo.json'
	condition: "and(succeeded(), eq(variables['BuildRTM'], 'false'))"

For e.g., https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=4389897&view=artifacts&pathAsName=false&type=publishedArtifacts (buildinfo.json has been published to build artifacts)

AFAIK, buildinfo.json is read from the file share in NuGet.Client Release pipeline. I propose following changes to the pipeline definition to read buildinfo.json file from build artifacts.

• Download buildinfo.json file from build artifacts. Change Insert into VS task as shown below. Thank you @dominoFire for teaching me on how to download build artifacts in the release pipeline.

• Change Get VS Target Branch script to read buildinfo.json from $(System.ArtifactsDirectory).

if(-not $env:VSTARGETBRANCH)
{
    $BuildInfoJsonFile = [System.IO.Path]::Combine('$(System.ArtifactsDirectory)','$(Build.DefinitionName)','BuildInfo','buildinfo.json')
//no change in the other lines of the inline script.
}
else
{
Write-Output "Using overridden branch $(VsTargetBranch)"
}

• Change Get Assembly Version script to read buildinfo.json from $(System.ArtifactsDirectory).

$BuildInfoJsonFile = [System.IO.Path]::Combine('$(System.ArtifactsDirectory)','$(Build.DefinitionName)','BuildInfo','buildinfo.json')
//no change in the other lines of the inline script.

• Save the pipeline.

Details:

Testing/Validation

Tests Added: No
Reason for not adding tests: Modified CI scripts.
Validation: Manual validation (Hopefully VS insertion completes successfully)

[kartheekp-ms]

buildinfo.json file was not created on file share but the file was uploaded to the build artifacts.

[nkolev92]

This change is fine, so approving.

I have more feedback about the release definition rather than this change in particular.

Can the release definition be updated so it downloads the file itself? Can we have an artifact that gets consumed by the release definition?

I'd like to avoid manual actions unless absolutely necessary.

Another thing to consider is that often we want to change the target branch, so adding the capability into the release definition natively might be a great thing.

[kartheekp-ms]

Can the release definition be updated so it downloads the file itself? Can we have an artifact that gets consumed by the release definition?
I'd like to avoid manual actions unless necessary.

The changes that I proposed to the release template in the PR description have to done one time.

Yes, artifacts can be downloaded by the release pipeline. Please refer to the screenshot in the PR description where I tried to explain how this can be achieved.

Another thing to consider is that often we want to change the target branch, so adding the capability into the release definition natively might be a great thing.

AFAIK, we have that capability to edit VS target branch in the current release definition. Refer to Get VS Target Branch inline script in the Insert into VS stage. The current process involves following steps.

• Create the release and select all Stages for a trigger change from automated to manual.
• Edit the VSTargetBranch release variables before triggering any stage.
• Trigger phases so that they can pick up the new target branch.

We can simplify the above steps by enabling an option to set VSTargetBranch variable while creating a new release as shown below.

[zivkan]

Since our builds have been publishing buildinfo.json as a build artifact for months/years, I suggest modifying the release pipeline before merging this PR.

[kartheekp-ms]

@zivkan - I modified the release pipeline. see commit history

[nkolev92]

We can simplify the above steps by enabling an option to set VSTargetBranch variable while creating a new release as shown below.

I think that'd be great.

Yes, artifacts can be downloaded by the release pipeline. Please refer to the screenshot in the PR description where I tried to explain how this can be achieved.

I think we should totally do that. I misread the description.