-
Notifications
You must be signed in to change notification settings - Fork 394
Make official build assetless and clean-up #9555
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
base: main
Are you sure you want to change the base?
Conversation
Contributes to dotnet/dotnet#711
MiYanni
left a comment
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.
CC @marcpopMSFT if this is desired to no longer have an official build (doesn't build the solution when running the pipeline) for templating.
| MirrorBranch: 'main' | ||
| JobNameSuffix: '_main' | ||
| condition: eq(variables['Build.SourceBranch'], 'refs/heads/main') | ||
| - template: /eng/common/templates-official/jobs/jobs.yml@self |
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.
What does this template do now that it has no jobs defined?
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.
Here's an example of a build pipeline with an assetless build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2841936&view=results
There's one job "Publish Assets" that inserts the build into BAR which results in the source code flow into the VMR. This intentionally doesn't build the repository anymore. In the future, later during .NET 11, we won't need a pipeline for this insertion anymore and Maestro will poll commits instead.
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.
Maybe I'm misunderstanding, but I thought "Publish Assets" publishes the assets from the build. Meaning, you need to build the repo to publish the assets from it. 🤔
| template: v1/1ES.Official.PipelineTemplate.yml@1esPipelines | ||
| ${{ else }}: | ||
| template: v1/1ES.Unofficial.PipelineTemplate.yml@1esPipelines | ||
| template: v1/1ES.Official.PipelineTemplate.yml@1ESPipelineTemplates |
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 yml is used in the templating unofficial pipeline so most of the changes to this file can't be taken as is. If we want to split into a third yml, I guess we can do that but we can't use the official pipelines in the unofficial yml and we need to have legs that actually do builds.
To be clear here, -pr is used for public PR builds on dnceng-public. The other yml is used for CI and for internal PRs. There was more different for internal versus public that we used the CI yml for internal PRs rather than using the PR yml for internal PRs.
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.
If we want to split into a third yml, I guess we can do that but we can't use the official pipelines in the unofficial yml and we need to have legs that actually do builds.
Yes, repositories which have an unofficial internal pipeline need a third entry-point YML file. Pushed a commit. We will need to update the unofficial pipeline post merge to point to the new YML.
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.
Yes, repositories which have an unofficial internal pipeline need a third entry-point YML file. Pushed a commit. We will need to update the unofficial pipeline post merge to point to the new YML.
That is not true. I use the same pipeline entry point for the Installer build currently that runs both the official build AND the unofficial build. dotnet/installer#20657
It is a myth that you need 2 entry points. You just need to know what should only exist within the official build and then conditionally remove it from the unofficial build. For triggers, you can edit the triggers for the unofficial pipeline within the pipeline itself, overriding what is declared in the YAML.
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.
By reusing that YML for both the official build (which has a vastly different layout) and the unofficial build, you will have tons of ifdefs and as you mentioned you would need to override triggers in the UI (for which you need special permissions). Those are all reasons why you want to have a separate file. At the end of the day, I don't have a strong opinion here and you own the repository so you decide. I'm just sharing best practices from my point of view.
@MiYanni maybe you can help us understand how a merged layout would look like in comparison to what I currently have in this PR.
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.
It isn't as many ifdefs as you might think if you design the pipeline for it. That's what I did for the SDK repo and intentionally thought about this situation at the onset. For templating, to be blunt, I don't care about this repo or its pipelines. At this point, my opinion is that we should merge this repo into the SDK repo and call it a day. But we don't have time/resources to do that currently. I'm fine with a separate pipeline here. But saying "it is required to make a new entry point" just isn't a true statement, is the main point I was trying to make. 😝 I only needed to change 4 lines in the Installer YAML to make it work.
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.
I believe it has cost us less to use one yml than it would to maintain two up to this point. I don't know if making the build assetless increases enough to justify an extra yml to maintain.
Contributes to dotnet/dotnet#711
Problem
Solution
Checks: