Page MenuHomePhabricator

Support changelist-based SVN workflows in Arcanist and Differential
Open, WishlistPublic

Description

User report:

Arcanist is great tool, but it uses whole SVN working copy data as CR. Our workflow relies heavy on svn's changelists and changes from different task are held under different changelists. I am able to extract neccesary diff's from working copy, but uploading them manually causes that Differential gets no context (ability to expand lines) in browsing review.
Arcanist on the other hand produces exactly same diff as I am, but somehow Differential catches context of changes from diff.
Even more - if I will download diff generated by arcanist, and upload it manually, then created review still have no context.
Can you guide me how I should upload diff data to Phabricator to support changelist-based workflow in SVN?

...this doesn't work well at all right now since we (Phacility) had never heard of this changelist feature for SVN until this report, so Arcanist is completely ignorant of it.

We can basically fake the feature in arc diff: arc diff x y z creates a revision with just the changes in those files, and then arc commit commits the changes in just those files. So you get a virtual changelist called "D123", sort of, that SVN doesn't know about.

Event Timeline

btrahan created this task.Mar 25 2014, 11:03 PM
btrahan raised the priority of this task from to Needs Triage.
btrahan updated the task description. (Show Details)
btrahan added subscribers: btrahan, epriestley.

Hi, I'm user who asked btrahan for this idea, so let me expand ;)

I'm attaching two files:

  • - 'svn status' with multiple changelists existing
  • - 'svn status --cl "Propozycja: Prywatne minicory dla taskworkerow"'

As you can see, it should be pretty simple to limit changes to single changelist instead of whole working copy.

If I could imagine "the perfect" workflow for this scenario, then "arc diff" should show me all changelists as first step, something like:

Which changelist would you like to use:
  0. Default changelist
  1. Changelist: cd3 rbt
  2. Changelist: Propozycja: Prywatne minicory dla taskworkerow
  3. Changelist: unittesty oraz pelne CD3

Select changelist [0]:

And I could select changelist by number instead of name. But I know that this would be 'perfect', and any other way would be sufficient too ;)

I've tried to hack Arcanist by forcing executing 'svn status --xml --cl "...."' in ArcanistSubversionAPI::getSVNStatus, but this try failed for now - I planned to do this durring invocation of ArcanistDiffWorkflow::generateAffectedPaths, but getSVNStatus is called earlier and I have to figure the proper way.

If you could hint me how I could achieve that, then I could try to create some patch and commit to my github repo. Or you could tell me to stay calm and do not hack anything ;)

HTH
Grzegorz Przydryga

rudykocur added a comment.EditedMar 27 2014, 7:59 PM

I've forked arcanist to my repo, and commited patch/fix to support changelist-based workflow. Please let me know what do you think about it

https://github.com/rudykocur/arcanist/commit/6832a05fb39d6da949a25a2ee6b9fc1471540453

Or maybe I should submit patch to Differential? ;)

@rudykocur , does the change you've made work as expected and no issues were found so far?

I believe that arc export should support changelists as well.

@epriestley, does changed proposed by @rudykocur look solid can can be merged upstream?

Pursuing this or reviewing the patch is a very low priority for us because use of changelists appears to be very rare among Phabricator users.

epriestley triaged this task as Wishlist priority.Oct 31 2014, 12:41 PM

It's ok if you don't merge it to the upstream (I can merge it my fork regardless of this). But what I'm asking if implementation is correct and code is following Phabricator coding guidelines and such. Maybe this needs to be unit tested somehow?

I haven't looked at it. Reviewing code takes time, and this feature isn't a priority.

Any updates on this feature? We are also quite a heavy svn changelist users as it helps a lot when creating more than one change on our working copy.

aik099 added a comment.EditedMay 4 2015, 12:35 PM

@oujesky, you can use my repo fork https://github.com/aik099/arcanist , that has such functionality via --cl argument arc diff, arc export and arc lint commands.

kfsone added a subscriber: kfsone.Mar 14 2017, 6:47 PM

@epriestley I'm in the process of setting up phabricator to help mature the development workflow based on svn here, and make future transitions to git/perforce easier. Changelists would be a natural path here but the workaround for our use case is to just manually specify paths, but it's annoying if you have a locally modified file that you accidentally include in a diff, since you *have* to revert it to remove it from the diff. Using a changelist would let you avoid making the mistake in the first place.