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 packages in topological order #11434

Merged
merged 28 commits into from Aug 24, 2022
Merged

Publish packages in topological order #11434

merged 28 commits into from Aug 24, 2022

Conversation

jenn-le
Copy link
Contributor

@jenn-le jenn-le commented Aug 4, 2022

This uses lerna ls --toposort in order to get the topological order of packages in the repo and changes the pipeline step to publish packages in this order.

Successful pipeline run here: https://dev.azure.com/fluidframework/internal/_build/results?buildId=83788&view=results

@jenn-le jenn-le requested a review from a team as a code owner August 4, 2022 23:45
@github-actions github-actions bot added area: build Build related issues base: main PRs targeted against main branch labels Aug 4, 2022
@jenn-le
Copy link
Contributor Author

jenn-le commented Aug 4, 2022

Questions:

  1. I noticed we do something different for the pack step when lerna isn't available. Is it necessary to publish in topological order for this scenario as well?
  2. I save the topological order to a file from the pack step so that I don't have to mess with the publish step's working directory in order to get access to lerna. Does this file need to be removed at the end?

@tylerbutler
Copy link
Member

  1. I noticed we do something different for the pack step when lerna isn't available. Is it necessary to publish in topological order for this scenario as well?

Sort of? If we're not using lerna, then it's an independent package (examples include protocol-definitions, tinylicious, etc.), and thus has its own CI pipeline to build and release. The release process itself should ensure that the releases are done in the right order, (for example, when releasing the client packages, if a protocol-defs update is needed, the release tools require it to be released first) so you shouldn't need to account for it.

  1. I save the topological order to a file from the pack step so that I don't have to mess with the publish step's working directory in order to get access to lerna. Does this file need to be removed at the end?

It will likely trigger the "Extraneous files check" in CI. That's designed to catch files that are generated unexpectedly during CI.

package_name=$(grep -oP 'npm notice package: \K.*' "publish_log")
npm pack $package_name
if cmp -s "$f" "$local_f"
f=$(find -name "${packageName}-[0-9]*.tgz")
Copy link
Member

@curtisman curtisman Aug 15, 2022

Choose a reason for hiding this comment

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

Does this work with our internal version schema 2.0.0-internal.x.y.z?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, since this is a glob pattern and not regex. This pattern expects a single number after the package name to get around some packages starting with the same names e.g. a-b-2.0.0.tgz and a-b-c-2.0.0.tgz. Then as long as the name ends in .tgz, the file will match.

@jenn-le
Copy link
Contributor Author

jenn-le commented Aug 15, 2022

  1. I noticed we do something different for the pack step when lerna isn't available. Is it necessary to publish in topological order for this scenario as well?

Sort of? If we're not using lerna, then it's an independent package (examples include protocol-definitions, tinylicious, etc.), and thus has its own CI pipeline to build and release. The release process itself should ensure that the releases are done in the right order, (for example, when releasing the client packages, if a protocol-defs update is needed, the release tools require it to be released first) so you shouldn't need to account for it.

  1. I save the topological order to a file from the pack step so that I don't have to mess with the publish step's working directory in order to get access to lerna. Does this file need to be removed at the end?

It will likely trigger the "Extraneous files check" in CI. That's designed to catch files that are generated unexpectedly during CI.

Thanks, @tylerbutler. Sorry for the late response, I wasn't notified you replied. For the second point, does this check get triggered in test pipelines? The pipelines I've run against this branch have been passing without seeming to trigger anything.

@tylerbutler
Copy link
Member

For the second point, does this check get triggered in test pipelines? The pipelines I've run against this branch have been passing without seeming to trigger anything.

@jenn-le If the CI is passing, then we'll call it good. :) I suggest merging main and pushing before you merge so you can get a fresh CI run.

@jenn-le
Copy link
Contributor Author

jenn-le commented Aug 24, 2022

For the second point, does this check get triggered in test pipelines? The pipelines I've run against this branch have been passing without seeming to trigger anything.

@jenn-le If the CI is passing, then we'll call it good. :) I suggest merging main and pushing before you merge so you can get a fresh CI run.

Here's a fresh, passing CI run: https://dev.azure.com/fluidframework/internal/_build/results?buildId=87938&view=results
I'll go ahead and merge this PR now.

@jenn-le jenn-le merged commit 1c82884 into main Aug 24, 2022
@jenn-le jenn-le deleted the test/topo-package-publish branch August 24, 2022 22:08
@github-actions
Copy link
Contributor

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

tyler-cai-microsoft pushed a commit to tyler-cai-microsoft/FluidFramework that referenced this pull request Aug 26, 2022
WayneFerrao pushed a commit to WayneFerrao/FluidFramework that referenced this pull request Aug 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: build Build related issues base: main PRs targeted against main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants