Weird behaviour of arc diff --verbatim
Closed, InvalidPublic
Actions

Assigned To

Authored By

	xclaesse
	Oct 16 2015, 7:06 PM

Description

I'm using git. I already created a Differential for my HEAD commit, but I updated it to fix review comments and changed the summary in the commit message. The commit already has "Differential Revision: D123".

I do "arc diff --verbatim", the doc says: "When updating a revision, update some fields from the local commit message."

But what happens is it creates a new Differential, and won't update the link in the commit message. As far as I understand, it should update existing differential instead, just like --edit. No?

My goal is to make it update the summary (and other fields, like Maniphest reference) on the website according to my new commit message.

Event Timeline

xclaesse created this task.Oct 16 2015, 7:06 PM

xclaesse updated the task description. (Show Details)

xclaesse added a project: Arcanist.

xclaesse updated the task description. (Show Details)

xclaesse added a subscriber: xclaesse.

I can't reproduce this. Here's what I did:

I created a change in a local branch.

$ git checkout -b patch1
Switched to a new branch 'patch1'
$ echo change >> README 
$ git commit -am change
[patch1 62e5984] change
 1 file changed, 1 insertion(+)

I ran arc diff to create a revision:

$ arc diff
...
Updating commit message...
Created a new Differential revision:
        Revision URI: http://local.phacility.com/D89

Included changes:
  M       README

This amended HEAD:

$ git show
commit 4906333a34e271ec4af20ae42465ec6eb1b507c7
Author: epriestley <git@epriestley.com>
Date:   Fri Oct 16 12:38:31 2015 -0700

    change
    
    Summary: summary
    
    Test Plan: test plan
    
    Reviewers: dog, saurus
    
    Subscribers: squeakybirdo
    
    Differential Revision: http://local.phacility.com/D89

diff --git a/README b/README
index 330fcf8..899c431 100644
--- a/README
+++ b/README
@@ -62,3 +62,4 @@ spaghetti
 hi
 spaghetti
 spaghetti
+change

I ran arc diff --verbatim to update it:

$ arc diff --verbatim
...
Updated an existing Differential revision:
        Revision URI: http://local.phacility.com/D89

Included changes:
  M       README

Note that it updated the revision properly.

$ arc diff
...
Created a new Differential revision:
      Revision URI: https://phabricator.collabora.co.uk/D239

$ git show

commit f68f082e60e3f10db14b9baad0ccd2256d199582
Author: Xavier Claessens <xavier.claessens@collabora.com>
Date:   Fri Oct 16 16:17:18 2015 -0400

    change
    
    Differential Revision: https://phabricator.collabora.co.uk/D239
...

Change my commit message:

$ git commit --amend --message change2

Update my differential to reflect my new commit:

$ arc diff --verbatim
...
Created a new Differential revision:
      Revision URI: https://phabricator.collabora.co.uk/D240

It created a new differential instead of updating the existing one.

Wait, no, doing it wrong, in that example it remove the link of course... let me test that properly again, seems I got confused :P

(11:27:44) avivey: do you have the "differential revision:" field in th emessage?
(11:27:50) xclaesse: yes

When you run git commit --amend --message change2, you replace the commit message with "change2", which doesn't have the "differential revision" field.

@avivey, right, that was just a quick test that I got wrong.

I understood why it wasn't working, looks like it's the commit msg parsing that gets confused.

$ git show
commit 1106a7af2f362cfa7e7f317dd8e50dd9d8079bb1
Author: Xavier Claessens <xavier.claessens@collabora.com>
Date:   Fri Oct 16 16:37:55 2015 -0400

  change
  
  Differential Revision: https://phabricator.collabora.co.uk/D244
  
  refs T2495

$ arc diff --verbatim
Created a new Differential revision:
      Revision URI: https://phabricator.collabora.co.uk/D245

If I swap refs and differential lines, then it works.

Ah, with "Maniphest Tasks:" it works fine.

Yeah, that's the way it works. It parses message as a form input, where the fields start with "Foo:". All text before the first field is assumed to be "title" and "summary".

We parse fields until we encounter the next field header because clients may linewrap, so the message above has a "Title" field with value change, and a "Differential Revision" field with value https://phabricator.collabora.co.uk/D244 refs T2495. This isn't a valid revision identifier so we can't figure out what to update.

You can use "Maniphest Tasks:" (a separate field header), or put it in your summary.

Ok, thanks for the explanation. I also sometimes set "Depends on D123" I guess it will make the same kind of parser issue. What's the syntax to have a propoer field header?

What's the syntax to have a propoer field header?

A valid field name, followed by a colon.

To find valid field names:

Load all subclasses of DifferentialCoreCustomField available on the server.
Iterate over them, calling getCommitMessageLabels().
The union of all results is the set of valid field names (case-insensitive).

xclaesse closed this task as Invalid.Oct 16 2015, 9:16 PM

xclaesse claimed this task.

Weird behaviour of arc diff --verbatimClosed, InvalidPublicActions

Description

Event Timeline

Weird behaviour of arc diff --verbatim
Closed, InvalidPublic
Actions