Specify sources directory explicitly in ADO pipelines#26529
Specify sources directory explicitly in ADO pipelines#26529ChumpChief merged 39 commits intomainfrom
Conversation
…FF repo where mandatory
…heckout location for repo-policy-check
There was a problem hiding this comment.
Pull request overview
This pull request addresses Azure DevOps pipeline issues related to repository checkout paths when multiple repositories are checked out. ADO changes the Build.SourcesDirectory location depending on whether one or multiple repos are checked out, which would break pipelines when conditionally checking out a second repository (needed for bundle size telemetry in internal runs).
Changes:
- Introduced new variables (
consistentSourcesDirectory,FluidFrameworkDirectory,FFPipelineHostDirectory) ininclude-vars.ymlto provide consistent checkout paths independent of the number of repos checked out - Updated all pipeline templates and build files to use rooted paths (
$(Pipeline.Workspace)/...) forworkingDirectoryparameters instead of relying onBuild.SourcesDirectory - Added explicit
checkout: selfsteps with path parameters to stages that previously relied on implicit checkouts - Fixed path handling in build scripts by updating environment variables and file paths to use the new consistent directory structure
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tools/pipelines/templates/include-vars.yml | Added new variables for consistent checkout paths across single/multi-repo scenarios |
| tools/pipelines/templates/upload-dev-manifest.yml | Added explicit checkout step with path; updated workingDirectory to use rooted paths |
| tools/pipelines/templates/include-test-task.yml | Updated workingDir for npm tasks to use rooted paths |
| tools/pipelines/templates/include-test-real-service.yml | Added TODO for pipelineHostPath that still uses Build.SourcesDirectory |
| tools/pipelines/templates/include-set-package-version.yml | Updated workingDirectory and filePath to use rooted paths |
| tools/pipelines/templates/include-publish-npm-package.yml | Removed unnecessary quotes from buildDirectory parameter |
| tools/pipelines/templates/include-process-test-results.yml | Updated searchFolder to use rooted paths |
| tools/pipelines/templates/include-policy-check.yml | Added explicit checkout step; updated all directory references |
| tools/pipelines/templates/include-install.yml | Updated workingDirectory to use rooted path |
| tools/pipelines/templates/include-install-pnpm.yml | Updated cache key path and workingDirectory to use rooted paths |
| tools/pipelines/templates/include-install-build-tools.yml | Updated buildDirectory and workingDirectory for build-tools installation |
| tools/pipelines/templates/include-build-lint.yml | Updated workingDir for build and lint tasks; fixed formatting |
| tools/pipelines/templates/build-npm-package.yml | Added checkout path; updated all workingDirectory, filePath, and sourceFolder references; added workingDirectory to extraneous files check; exposed new variables to template context |
| tools/pipelines/templates/build-npm-client-package.yml | Similar updates as build-npm-package.yml plus devtools path fixes; fixed WORKING_DIRECTORY env var for pack-build-output.sh |
| tools/pipelines/repo-policy-check.yml | Duplicated variable definitions from include-vars.yml; added Variables display task and explicit checkout |
| tools/pipelines/deploy-website.yml | Added TODO comments for buildDirectory parameters that still use Build.SourcesDirectory |
| tools/pipelines/build-test-tools.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-protocol-definitions.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-eslint-plugin-fluid.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-eslint-config-fluid.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-docs.yml | Added TODO comments for buildDirectory that still uses Build.SourcesDirectory |
| tools/pipelines/build-common-utils.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-client.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-bundle-size-artifacts.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-build-tools.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-build-common.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-benchmark-tool.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/build-api-markdown-documenter.yml | Updated buildDirectory to use FluidFrameworkDirectory variable |
| tools/pipelines/templates/upload-telemetry/include-stage-upload-telemetry.yml | Added TODO for buildDirectory that still uses Build.SourcesDirectory |
d4d513c to
188e9a9
Compare
alexvy86
left a comment
There was a problem hiding this comment.
Overall I agree with the direction. Just one thing:
buildDirectory parameter - this remains a relative path, but relative to the Pipeline.Workspace variable (rather than relative to the SourcesDirectory).
We're not using it for the checkout path which you mention cannot be an absolute one, so would it be better to make buildDirectory absolute everywhere? We could even have small validation steps as the first thing in any template that has such a parameter to ensure that whatever is passed is an absolute path. Seems easier to enforce that invariant "at the top" instead of having to prepend $(Pipeline.Workspace) everywhere we use it.
That's a good idea, I'll take a look. I think I started off in this direction as an attempt to minimize the delta to current behavior, but agree that seems preferable. |
@alexvy86 Relative path is still used in build-docker-service to locate the test coverage report relative to the ArtifactStagingDirectory - should be solvable but I'd probably prefer to stick with the relative paths for initial checkin at least. |
alexvy86
left a comment
There was a problem hiding this comment.
LGTM as far as one can tell with pipeline changes just by looking at them.
AB#60000
ADO Pipelines changes the location of checked-out repos depending on whether a single vs. multiple repos are checked out. As a result, checking out a second repo would break most of our pipelines. To fix the broken bundle size telemetry, we want to !conditionally! check out a second repo in "internal" runs, which also exacerbates this quirk.
Fix here is to be explicit about checkout locations for the repo (see include-vars.yml) such that it will be consistent no matter how many repos are checked out, and then fix up a bunch of places that make assumptions about install location. Broadly, these are:
$(Build.SourcesDirectory)/scripts/..., these are made to point more explicitly to the FF repo checkout rather than just the SourcesDirectory or buildDirectory.It is difficult to confirm that I've updated all permutations and run options that are touched by this change, but I've manually run a wide variety in various configurations - I'll be sure to remain available to monitor following merge in case I've missed anything.