Fetch all history for all tags and branches when fetch-depth=0#258
Fetch all history for all tags and branches when fetch-depth=0#258ericsciple merged 1 commit intomasterfrom
Conversation
| branches = await git.branchList(true) | ||
| for (const branch of branches) { | ||
| await git.branchDelete(true, branch) | ||
| // Remove any conflicting refs/remotes/origin/* |
There was a problem hiding this comment.
Detects two types of conflicts:
Example 1: Consider ref is refs/heads/foo and an existing ref refs/remotes/origin/foo/bar exists
Example 2: Consider ref is refs/heads/foo/bar and an existing ref refs/remotes/origin/foo exists
| } | ||
|
|
||
| async shaExists(sha: string): Promise<boolean> { | ||
| const args = ['rev-parse', '--verify', '--quiet', `${sha}^{object}`] |
There was a problem hiding this comment.
rev-parse magic :)
^{object} means the object can be any type
There was a problem hiding this comment.
i love git :)
it's a great command line tool that lets you do what you need to do
| async fetch(refSpec: string[], fetchDepth?: number): Promise<void> { | ||
| const args = ['-c', 'protocol.version=2', 'fetch'] | ||
| if (!refSpec.some(x => x === refHelper.tagsRefSpec)) { | ||
| args.push('--no-tags') |
There was a problem hiding this comment.
not adding --no-tags when +refs/tags/*:refs/tags/* is part of the refspec... i think it would technically work the same, but would just look weird
| - uses: actions/checkout@v2 | ||
| ``` | ||
|
|
||
| ## Fetch all tags |
There was a problem hiding this comment.
bye bye complicated stuff
| - [Fetch all branches](#Fetch-all-branches) | ||
| - [Fetch all history for all tags and branches](#Fetch-all-history-for-all-tags-and-branches) | ||
|
|
||
| ## Fetch all history for all tags and branches |
There was a problem hiding this comment.
this comment sums up the motivation nicely:
Seems pretty logical and reasonable to me that if I set fetch-depth: 0 then I clearly want everything and history matters to me and that you should not add the --no-tags parameter.
| } | ||
|
|
||
| export function getRefSpecForAllHistory(ref: string): string[] { | ||
| const result = ['+refs/heads/*:refs/remotes/origin/*', tagsRefSpec] |
There was a problem hiding this comment.
branches and tags
| const result = ['+refs/heads/*:refs/remotes/origin/*', tagsRefSpec] | ||
| if (ref) { | ||
| const upperRef = (ref || '').toUpperCase() | ||
| if (ref.toUpperCase().startsWith('REFS/PULL/')) { |
There was a problem hiding this comment.
and conditionally the pull request branch
f250f1b to
759d29e
Compare
| // Example 1: Consider ref is refs/heads/foo and previously fetched refs/remotes/origin/foo/bar | ||
| // Example 2: Consider ref is refs/heads/foo/bar and previously fetched refs/remotes/origin/foo | ||
| if (ref) { | ||
| ref = ref.startsWith('refs/') ? ref : `refs/heads/${ref}` |
There was a problem hiding this comment.
Should we do the same thing with tags? In case a force push has been made to a tag, a pull wouldn't work.
There was a problem hiding this comment.
Note, this PR doesn't change that behavior but I did think about tags too.
Technically could happen for tags, however unlikely because tags typically aren't pathy and also commonly never move. Further evidence: we don't handle today and it hasn't been a problem. Same during azp, never saw issue for tag.
If it were to happen here is what the error looks like:
$ git tag pathy/name
$ git tag pathy
fatal: cannot lock ref 'refs/tags/pathy': 'refs/tags/pathy/name' exists; cannot create 'refs/tags/pathy'
Delete the repo and reclone is one way to fix. Otherwise when fetching all history, will be deleted because of git fetch origin --prune '+refs/tags/*:refs/tags/*'
|
|
||
| // When all history is fetched, the ref we're interested in may have moved to a different | ||
| // commit (push or force push). If so, fetch again with a targeted refspec. | ||
| if (!(await refHelper.testRef(git, settings.ref, settings.commit))) { |
There was a problem hiding this comment.
print to console?
| } | ||
| // Unexpected | ||
| else { | ||
| core.debug(`Unexpected ref format '${ref}' when testing ref info`) |
There was a problem hiding this comment.
do we need to error on unexpected?
There was a problem hiding this comment.
if i had confidence from telemetry i would error :)
|
Note to self, tested E2E:
|
bd912f9 to
bbe3883
Compare
Signed-off-by: Eric Promislow <epromislow@suse.com>
Fetch all history for all tags and branches when
fetch-depth: 0Fixes #240
Fixes #239
Fixes #214
Fixes #204
Fixes #93
Related #250
Related #249
Related #217