1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-12-12 00:26:12 +01:00
Commit graph

3 commits

Author SHA1 Message Date
epriestley
53691cdcf9 Fix an escaping issue with "svn commit"
Summary: Fixes T2438. We currently escape everything with '@', but SVN rejects that for '.'

Test Plan:
Unit tests. Performed this commit:

  $ svn st
   M      .
  A       x@123
  $ arc commit --conduit-uri=http://local.aphront.com:8080/ --revision 53

      Revision 'D53: asdf' has not been accepted. Commit this revision anyway?
      [y/N] y

  Committing 'D53: asdf'...
  Sending        .
  Adding         x@123
  Transmitting file data .
  Committed revision 37.
  Done.

I grepped for more '@' adding but couldn't find any. It's a bit tricky to grep for though, so it's possible I missed some.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, mbishopim3

Maniphest Tasks: T2438

Differential Revision: https://secure.phabricator.com/D4703
2013-01-28 14:11:31 -08:00
epriestley
2ae0cb797d Remove ArcanistRepositoryAPI::setDefaultBaseCommit()
Summary:
This method is used in three cases:

  # For unit tests, to set the range to 'HEAD^' or '.^' in an agnostic way.
  # For "amend", to set the range to the commit to be amended (also 'HEAD^' or '.^').
  # For "patch" and "upgrade" so we don't fail just because there's an invalid "base" rule somewhere in the config when doing clean-working-copy tests.

For cases (1) and (2), introduce an "arc:this" rule to mean "the current commit". For case (3), remove the call; it is no longer necessary to check the commit range in order to do tests for the working copy state after D4095.

Test Plan: Ran unit tests, "arc upgrade", "arc patch", "arc amend".

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4096
2012-12-17 12:53:38 -08:00
epriestley
0e27dbfd17 Refactor getWorkingCopyStatus() into getUncommittedStatus() and getCommitRangeStatus()
Summary:
See discussion in D4049.

The getWorkingCopyStatus() method gets called from requireCleanWorkingCopy() in a lot of places, which triggers resolution of the base of the commit range. This is unnecessary; we do not need to examine the base commit in order to determine whether the working copy is dirty or not. This causes problems, in D4049 and elsewhere (we currently have a lot of fluff calls to setDefaultBaseCommit() in workflows which need to call requireCleanWorkingCopy() but do not ever use commit ranges, such as `arc patch`). This is mostly an artifact of SVN, where the "commit range" and "uncommitted stuff in the working copy" are always the same.

  - Split the method into two status methods: getUncommittedStatus() (uncommitted stuff in the working copy, required by requireCleanWorkingCopy()) and getCommitRangeStatus() (committed stuff in the commit range).
  - Lift caching out of the implementations into the base class.
  - Dirty the cache after we commit.

This doesn't do anything useful on its own and creates one caching problem (`commitRangeStatusCache` is not invalidated when the commit range changes because of `setBaseCommit()` or similar) but I wanted to break things apart here. I won't land it until there's a more complete picture.

This creates a minor performance regression in git and hg (we run less stuff in parallel than previously) but all the commands should be disk-bound anyway and the regression should be minor. It prevents a larger regression in `hg` in D4049, and lets us do less work to arrive at common error states (dirty working copy). We can examine perf at the end of this change sequence.

Test Plan: Ran unit tests, various `arc` commands.

Reviewers: vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D4095
2012-12-17 12:53:28 -08:00