Skip to content

add parseMergeResult functionality and tests to detect merge conflicts #5493

Merged
shiftkey merged 10 commits intomasterfrom
parsing-merge-tree-result
Sep 4, 2018
Merged

add parseMergeResult functionality and tests to detect merge conflicts #5493
shiftkey merged 10 commits intomasterfrom
parsing-merge-tree-result

Conversation

@shiftkey
Copy link
Member

This PR is the underlying Git work to support #4588. I generated a bunch of test data by scanning the desktop/desktop, electron/electron and microsoft/vscode repositories for examples of merge conflicts - these are now baked as tests into the project, and caught some corner cases I hadn't uncovered with the original code.

For the moment parseMergeResult just scans the Git output to build up an array of entries, to then determine what files have conflict. But there's already details in the diff output about how files conflict that we can use down the track.

For reference, this is how I generated the merge conflict files - if there's interest in me adding this as something more reusable to regenerate tests cases when we upgrade dugite let me know and we can figure out how to do that:

  const repo = new Repository('/Users/shiftkey/src/vscode', -1, null, false)
  const allBranches = await getBranches(repo)
  const start =
    allBranches.find(b => b.nameWithoutRemote === 'master') || null

  if (start == null) {
    throw new Error('Unable to find current branch!')
  }

  await checkoutBranch(repo, null, start)

  const status = await getStatusOrThrow(repo)

  if (status.currentBranch == null) {
    throw new Error('Unable to find current branch!')
  }

  const ours =
    allBranches.find(b => b.nameWithoutRemote === status.currentBranch) ||
    null

  if (ours == null) {
    throw new Error('Unable to find ours!')
  }

  const otherBranchesToCompare = allBranches.filter(
    b => b.tip.sha !== ours.tip.sha
  )

  for (const theirs of otherBranchesToCompare) {
    const fileName = `merge-${theirs.nameWithoutRemote}-into-${
      ours.name
    }.txt`.replace('/', '-')

    const mergeBase = await getMergeBase(repo, ours.tip.sha, theirs.tip.sha)

    if (mergeBase === ours.tip.sha) {
      continue
    }

    if (mergeBase === theirs.tip.sha) {
      continue
    }

    const result = await GitProcess.exec(
      ['merge-tree', mergeBase, ours.tip.sha, theirs.tip.sha],
      repo.path,
      { maxBuffer: 100 * 1024 * 1024 }
    )
    const stdout = result.stdout

    if (stdout.indexOf('<<<<<<') === -1) {
      continue
    }

    const relativePath = Path.join(
      __dirname,
      `../../fixtures/merge-parser/vscode/${fileName}`
    )

    await FSE.writeFile(relativePath, stdout)
  }
})

@shiftkey shiftkey added ready-for-review Pull Requests that are ready to be reviewed by the maintainers infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop labels Aug 28, 2018
@shiftkey shiftkey force-pushed the parsing-merge-tree-result branch from b1f8caa to 27afdc8 Compare August 28, 2018 15:22
ours: Branch,
theirs: Branch
): Promise<MergeResult | null> {
console.time('getMergeBase')

This comment was marked as spam.

@shiftkey shiftkey added this to the 1.4.0 milestone Aug 28, 2018
@shiftkey shiftkey mentioned this pull request Aug 30, 2018
/**
* Find the base commit between two refs
*
* @returns the commit id of the merge base, or null if the two refs do not

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

'merge-base'
'merge-base',
{
successExitCodes: new Set([0, 1]),

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

}

if (mergeBase === ours.tip.sha) {
return { kind: MergeResultKind.Success, entries: [] }

This comment was marked as spam.

function updateCurrentMergeEntry(
entry: IMergeEntry | undefined,
context: string,
blobSource: {

This comment was marked as spam.

@iAmWillShepherd iAmWillShepherd self-assigned this Aug 31, 2018
Copy link
Contributor

@iAmWillShepherd iAmWillShepherd left a comment

Choose a reason for hiding this comment

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

Only thing needed is some unit tests, is that something you want to do in a separate PR, so this one wont be held up?

@shiftkey shiftkey merged commit bf7259d into master Sep 4, 2018
@shiftkey shiftkey deleted the parsing-merge-tree-result branch September 4, 2018 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Issues and pull requests related to scripts and tooling for GitHub Desktop ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants