Page MenuHomePhabricator

Parse multiple commits and commit metadata from "hg log --patch" and "hg export"
Open, NormalPublic

Description

When creating a diff using differential.createrawdiff there's no way for the created diff in Phabricator to know what base revision the diff/commit should be based on, and there's no way to modify the created diff to set that base revision. Without knowing the base revision, when patching down the diff Arcanist attempts to apply it to the working set.

Problem Description

I'm creating a Mercurial hook for a repository (not hosted by Phabricator). This hook is used to monitor commits pushed to the repository for changes to specific files. Upon identifying modifications, the hook creates a new Revision with a new Diff of only the modifications to the specific files and adds a reviewer group onto the new revision. This revision will then be reviewed (changes requested), then patched down by someone, modifications made, reviewed, landed, etc.

The issue is that when the "patched down by someone" happens, Arcanist applies the patch to the current working set instead of the parent which the original commit was made under. This will rarely be correct, and depending on the circumstance the patch will apply cleanly (i.e. not error from mercurial) which causes the diff to be applied to be incorrect.

If there is a way for the diff created by createrawdiff to have its base revision set, this would cause Arcanist to apply the patch to the appropriate changeset in the local repository.

Event Timeline

A maybe-less-complicated approach here could be:

  • Support parsing metadata from hg log --patch --rev <rev>, ala similar planned changes for Git rich formats in T12256.
  • Pipe that into differential.createrawdiff.

This doesn't require me to ask any troublesome existential questions about HookDifferential workflows.

I tried to not specifically ask for a change to API or parsing, just that the missing bit I need is a base revision set on the new diff.

This doesn't require me to ask any troublesome existential questions

Oh, you're right. I misread an implication of implementation specifics into "set", but the actual language doesn't actually suggest an implementation.

OSMeteor removed a subscriber: OSMeteor.

@epriestley - Just to clarify, with the solution you're suggesting that would mean the diff created using differential.createrawdiffdifferential.revision.edit would result in arc patch of that revision applying the patch to the base revision parsed from the raw diff from hg log --patch --rev <rev>?

Yes. We'd retain the base revision information during parsing in differential.createrawdiff, and it would remain available later in arc patch.

epriestley renamed this task from Create a revision/diff using Conduit which sets the base revision to Parse multiple commits and commit metadata from "hg log --patch" and "hg export".May 2 2017, 4:47 PM
epriestley triaged this task as Normal priority.
epriestley added a project: Arcanist.

Parsing hg export metadata is an elegant solution. # HG changeset patch could imply sourceControlSystem = 'hg'. Thanks for merging the task!

epriestley moved this task from Backlog to Diff Parsing on the Arcanist board.Mar 5 2018, 2:16 PM