Page MenuHomePhabricator
Create Task
Maniphest T10869

`arc test` sometimes picks an older (incorrect) parent to apply patch to
Closed, InvalidPublic
Actions

Description

  1. check the list of common fixes below: We complete all these steps every time we upgraded.
  2. make sure Phabricator is up to date: Phabricator was last updated on 4/15/16
  3. make sure we support your setup: Uh, I think you do?
  4. gather debugging information: we initially recorded this issue internally in March 2015, but ran into it again this month. We experience this issue with arc test and arc diff. Not sure what would be helpful here ... no relevant error messages.
  5. explain how to reproduce the issue:

Here's what they did:

git checkout 297900e
git checkout -b oldhead
arc patch --diff 346688
arc test server-selenium      # <---- fails

Here's what the git graph looked like:

* 79e4d22 (HEAD, arcpatch-D97340) [omitted]
* 297900e (oldhead) [omitted]
* ace441a [omitted]
* 85e77c6 [omitted]

What was expected: When checking out a base and trying to apply a patch, we expect this to run based on the actual base of this branch (about a month old).

What actually happened: arc test tries to apply the patch against 1706371 (a commit from today!) .

Related Objects

Event Timeline

angie created this task.Apr 25 2016, 11:07 PM
Herald added a subscriber: jhurwitz. · View Herald TranscriptApr 25 2016, 11:07 PM
chad added a project: Arcanist.Apr 25 2016, 11:13 PM
avivey added a subscriber: avivey.Apr 25 2016, 11:24 PM
  • arc patch attempts to apply the patch to the commit the diff was based off; Is that what you were expecting?
  • arc test is not a command I have; What is it?
epriestley closed this task as Invalid.Apr 26 2016, 12:33 AM
epriestley added a subscriber: epriestley.

Yeah, arc test is not part of the upstream.

See T9181 for a previous report of a problem with arc test which also had nothing to do with the upstream.

If there's some underlying bug here that really is our fault, please resubmit reproduction instructions based only on upstream code. I can't possibly reproduce anything which requires running arc test because that command doesn't exist in the upstream.

epriestley added a comment.Apr 26 2016, 12:35 AM

Specifically, any effort to reproduce this in the upstream will meet this grizzly fate on the arc test step:

$ arc test
Usage Exception: Unknown command 'test'. Try 'arc help'.
angie added a comment.Apr 26 2016, 12:35 AM

As I mentioned, we also experience this with arc diff. Is that not supported?

epriestley added a comment.Apr 26 2016, 12:42 AM

arc diff is supported, but most of these reproduction instructions don't make much sense to me in the context of arc diff -- for example, arc diff does not apply patches.

I'll make an effort to reproduce this anyway, but this looks a lot like a fishing expedition.

epriestley added a comment.Apr 26 2016, 12:49 AM

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

I checked out an old commit, then created a branch from it:

$ git checkout HEAD^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
HEAD is now at 025b243... Document wiki relative link syntax
$ git checkout -b oldhead
Switched to a new branch 'oldhead'

I applied a patch with arc patch --diff ...:

$ arc patch --diff 37750
Created and checked out branch arcpatch-D15664.
...

Note that this automatically creates a new branch for the patch based on the correct base revision for the diff. I'm now on arcpatch-D15664 which has a different HEAD^ unrelated to oldhead. I could have applied it directly to oldhead with arc patch --nobranch ... instead.

I ran arc which to understand what arc diff will do when I run it. This is what I expect:

$ arc which
REPOSITORY
To identify the repository associated with this working copy, arc followed this process:

    Configuration value "repository.callsign" is empty.

    This repository has no VCS UUID (this is normal for git/hg).

    The remote URI for this working copy is
    "ssh://dweller@secure.phabricator.com/diffusion/P/".

    Found a unique matching repository.

This working copy is associated with the Phabricator repository.

COMMIT RANGE
If you run 'arc diff', changes between the commit:

    HEAD^  Normalize case on active operations

...and the current working copy state will be sent to Differential, because
HEAD has been amended with 'Differential Revision:', as specified by
'arc:amended' in your user 'base' configuration.

You can see the exact changes that will be sent by running this command:

    $ git diff 'HEAD^..HEAD'

These commits will be included in the diff:

    501c4308b967ab18  Increase specificity in blue property boxes


MATCHING REVISIONS
These Differential revisions match the changes in this working copy:

    D15664 (chad) Increase specificity in blue property boxes
        Reason: Commit message for '501c4308b967ab18' has explicit 'Differential Revision'.

Since exactly one revision in Differential matches this working copy, it will
be updated if you run 'arc diff'.

Finally, I run arc diff, which produces the correct changes:

$ arc diff --only
...
 PUSH STAGING  Pushing changes to staging area...
# Push received by "secure002.phacility.net", forwarding to cluster host.
# Waiting up to 120 second(s) for a cluster write lock...
# Acquired write lock immediately.
# Waiting up to 120 second(s) for a cluster read lock on "secure001.phacility.net"...
# Acquired read lock immediately.
# Device "secure001.phacility.net" is already a cluster leader and does not need to be synchronized.
# Ready to receive on cluster host "secure001.phacility.net".
Counting objects: 13, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (12/12), done.
Writing objects: 100% (13/13), 1.46 KiB | 0 bytes/s, done.
Total 13 (delta 10), reused 0 (delta 0)
# Released cluster write lock.
To ssh://dweller@secure.phabricator.com/diffusion/STAGING/staging.git
 * [new tag]         437ff2a718143e68e9b16cf86640fd492c8dca0a -> phabricator/base/38056
 * [new tag]         501c4308b967ab18e572d763daf875cc662b1345 -> phabricator/diff/38056
Created a new Differential diff:
        Diff URI: https://secure.phabricator.com/differential/diff/38056/

Included changes:
  M       resources/celerity/map.php
  M       webroot/rsrc/css/application/countdown/timer.css
  M       webroot/rsrc/css/phui/phui-box.css

The resulting diff is identical to the original:

epriestley added a comment.Apr 26 2016, 1:59 AM

Here are a few ideas that might shed some light on what's happening:

  • Is the --diff being applied special in any way? For example: not attached to a revision? Raw copy-pasted diff with no base revision which wasn't generated with arc diff?
  • Does arc patch not check out a branch? What's the output? Normally, it should create a new branch, but it may not be doing so if the diff is unusual.
  • What is the base rule being used (normally from arc get-config base)?
  • What does arc which show from the state that causes arc diff to misbehave?
Phabricator · Built by Phacility