Enable publishing build artifacts in PR

[carlossanlop]

Fixes #164

[carlossanlop]

The artifacts are getting successfully published: https://dev.azure.com/dnceng-public/public/_build/results?buildId=870307&view=artifacts&pathAsName=false&type=publishedArtifacts

But there might be bugs:

• There's a duplicate "Publish logs" step.
• Artifacts_WindowsNT_Release and Artifacts_WindowsNT_Debug are only publishing the packages of the assemblies that currently have IsPackable set to true (which is only S.R.CS.Unsafe). Artifacts_Linux_Release is the only one publishing all artifacts for all assemblies.
• In Artifacts_Linux_Release, there are artifacts being published for test projects.

Before vs After.

[carlossanlop]

Bug investigation:

---

There's a duplicate "Publish logs" step.

Seems to be expected. They are two different tasks, but the name happens to be the same.

• The first one is: "Publish Pipeline Artifacts" - https://dev.azure.com/dnceng-public/public/_build/results?buildId=870307&view=logs&j=a751307a-bba0-598c-20e2-725c5e776a43&t=cc34745a-1308-5e4e-a99f-df56644864c6

  • Description: Publish (upload) a file or directory as a named artifact for the current run

• The second one is: "Publish build artifacts" - https://dev.azure.com/dnceng-public/public/_build/results?buildId=870307&view=logs&j=a751307a-bba0-598c-20e2-725c5e776a43&t=ec181c62-f2b4-5264-b63d-2052c579474f

  • Description: Publish build artifacts to Azure Pipelines or a Windows file share

---

Artifacts_WindowsNT_Release and Artifacts_WindowsNT_Debug are only publishing the packages of the assemblies that currently have IsPackable set to true (which is only S.R.CS.Unsafe). Artifacts_Linux_Release is the only one publishing all artifacts for all assemblies.

Expected. That's how we configured them. Only the 3rd one is meant for packing everything (it passes /p:IsPackable=true, the others don't).

---

In Artifacts_Linux_Release, there are artifacts being published for test projects.

Seems that the Pack target gets always executed for all projects.

[ViktorHofer]

This is uploading too much. We don't want the bin folder i.e.:

[carlossanlop]

@michaelgsharp @ViktorHofer do we need to skip uploading artifacts when the CI is green? Is that done automatically?
Here are a couple of examples from machinelearning:

Green

• PR https://github.com/dotnet/machinelearning/pull/7312/checks?check_run_id=33011627816
• Build says "0 artifacts" https://dev.azure.com/dnceng-public/public/_build/results?buildId=870279&view=results

Red

• PR https://github.com/dotnet/machinelearning/pull/7314/checks?check_run_id=33051447181
• Build says "2 published" https://dev.azure.com/dnceng-public/public/_build/results?buildId=870926&view=results

[carlossanlop]

This is uploading too much. We don't want the bin folder i.e.:

@ViktorHofer I suppose there cases where we do want to upload the bin folder? For example, when we are not packing?

Here's an example of machinelearning, which only has bin and log folders: https://dev.azure.com/dnceng-public/public/_build/results?buildId=870926&view=artifacts&pathAsName=false&type=publishedArtifacts

Edit: Oh I see, machinelearning needs to publish the bin folder because it gets used in subsequent jobs, like in runtime. We don't do that here.

In that case, we have 3 different jobs in m-p: two are linux, one is windows. Only one of those 3 (a linux job) publishes packages. Do I need to disable publishing the bin folder for all 3?

[carlossanlop]

@ViktorHofer since we are building and packing in the same job, it makes sense that both the bin and packages folders get published.

Arcade does not offer an option to choose what to copy so that can then be published. It's both bin and packages, or none:

maintenance-packages/eng/common/templates/job/job.yml

Lines 188 to 205 in 8272de8

	- task: CopyFiles@2
	displayName: Gather binaries for publish to artifacts
	inputs:
	SourceFolder: 'artifacts/bin'
	Contents: '**'
	TargetFolder: '$(Build.ArtifactStagingDirectory)/artifacts/bin'
	- task: CopyFiles@2
	displayName: Gather packages for publish to artifacts
	inputs:
	SourceFolder: 'artifacts/packages'
	Contents: '**'
	TargetFolder: '$(Build.ArtifactStagingDirectory)/artifacts/packages'
	- task: PublishBuildArtifacts@1
	displayName: Publish pipeline artifacts
	inputs:
	PathtoPublish: '$(Build.ArtifactStagingDirectory)/artifacts'
	PublishLocation: Container
	ArtifactName: ${{ coalesce(parameters.artifacts.publish.artifacts.name , 'Artifacts_$(Agent.Os)_$(_BuildConfig)') }}

My option would be to rewrite this. Do we really want that?

[carlossanlop]

OK I fixed it @ViktorHofer ! No more bin
https://dev.azure.com/dnceng-public/public/_build/results?buildId=871196&view=artifacts&pathAsName=false&type=publishedArtifacts

[ViktorHofer]

@carlossanlop are you sure? I still see the bin folders under artifacts. We don't want or need those. The packages are enough. We don't upload the bin folders in other repos either.

[carlossanlop]

Oh, for a moment I thought the bin folder was shoing up in the root folder. My bad.

What I did remove was the duplicate "Log" folder.

[ericstj]

Looks to me like the relevant portion of the yml templates is here:

maintenance-packages/eng/common/templates/job/job.yml

Line 186 in cf6c8aa

- ${{ if ne(parameters.artifacts.publish, '') }}:

[carlossanlop]

Looks to me like the relevant portion of the yml templates is here:

maintenance-packages/eng/common/templates/job/job.yml

Line 186 in cf6c8aa

- ${{ if ne(parameters.artifacts.publish, '') }}:

I already tried re-writing that code: 97879c7

But I had to revert the commit as it was not working. But let me combine it with the latest changes I made, I must have missed something.

[ViktorHofer]

But I had to revert the commit as it was not working. But let me combine it with the latest changes I made, I must have missed something.

I don't think that Arcade makes this possible by default. You could contribute to Arcade's template to allow only publishing packages or add a bit of YML as most other repos do to upload the packages folder.

[carlossanlop]

This isn't urgent and the PR needs additional changes coming from the other repo. I'll close this for the time being.