Fix handling of shallow clones when checking for change files #780
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ecraig12345
force-pushed
the
ecraig/checkchange
branch
2 times, most recently
from
106564f
to
8ebf412
Compare
kenotron
reviewed
Sep 21, 2022
src/changefile/getChangedPackages.ts
Outdated
| // Fetch more history (if needed, this could be optimized later to only deepen by e.g. 100 commits at a time) | ||
| try { | ||
| console.log('This is a shallow clone. Unshallowing to check for changes...'); | ||
| git(['fetch', '--unshallow'], { cwd, throwOnError: true }); |
There was a problem hiding this comment.
Can you confirm this "throwOnError" flag? I can't find doc for it: https://nodejs.org/api/child_process.html#child_processspawnsynccommand-args-options
There was a problem hiding this comment.
Ah it's added in microsoft/workspace-tools#193 which isn't actually merged yet (I was testing this branch against a tgz of those changes at first)
There was a problem hiding this comment.
I got rid of the part that required the workspace-tools update
ecraig12345
force-pushed
the
ecraig/checkchange
branch
from
f8ce0cc
to
ae98556
Compare
kenotron
approved these changes
Sep 22, 2022
rajsite
added a commit
to ni/nimble
that referenced
this pull request
Mar 6, 2024
# Pull Request ## 🤨 Rationale - Updated `fetch-depth: 0` comment from being [needed by beachball](microsoft/beachball#780) to being [needed by chromatic](https://github.com/ni/nimble/actions/runs/8164295790/job/22319288123#step:14:50). - Updated renovate to make a single PR for npm updates that we can let update to latest. - Angular and it's related packages are ignored - `^@angular`, `ng-packagr`, `zone.js`, `typescript` - Pinned packages are ignored - `@microsoft/fast-foundation` and `@microsoft/fast-react-wrapper` due to [fast issue](microsoft/fast#6906) - `comlink` as it is partially vendored into workers and needs a controlled and pinned update - `remark-gfm` as [updating breaks mdx](storybookjs/storybook#24743 (comment)) - Removed npm version specific install that was now downgrading the npm version accidentally. - Removed file permission workaround [no longer needed to prevent warnings](actions/upload-pages-artifact#45). - Updated the beachball patch-package ## 👩💻 Implementation See above. ## 🧪 Testing Relying on CI. ## ✅ Checklist - [x] I have updated the project documentation to reflect my changes or determined no changes are needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
By default, github
actions/checkoutonly fetches the current merge ref of the PR (no history and no other branches). This causes problems forbeachball check, because it doesn't have all the history that it needs to compare against.These problems have existed for quite some time, in any repo using
actions/checkout> v1 withoutfetch-depth: 0. And theworkspace-toolsgit changed files helpers were hiding the issue by returning empty arrays if git commands failed.Fix is to add explicit checks on the beachball side for various conditions related to missing history. If history is missing and fetching is enabled, it will fetch the extra history. If fetching is disabled, it will print informative error messages.
(You can see that it works because this PR's initial build includes the log about unshallowing, and ultimately failed due to a missing change file!)