What about being able to git fetch a patch that is still in review. Is that already supported upstream and just not shown in the UI?

In T127#1083121, @JanZerebecki wrote:

What about being able to git fetch a patch that is still in review. Is that already supported upstream and just not shown in the UI?

It is supported

In T127#1083262, @chasemp wrote:

In T127#1083121, @JanZerebecki wrote:

What about being able to git fetch a patch that is still in review. Is that already supported upstream and just not shown in the UI?

It is supported

But see T136: Pulling patches from Phabricator does not give consistent commit hashes (people might get used to this in practice, not sure).

For the record, there's a differential upstream that would allow someone to create a new Differential by just pushing (using plain git) a new commit to a magic branch ('review'). It isn't landed (committed) yet, but it's there and working/testable.

In T5000#75467, @epriestley wrote:

D9599 allows you to create a diff by running git push origin HEAD:review (i.e., a git command) instead of arc diff (i.e., an arc command). This is similar to how Gerrit works.

Can the priority be changed since we're moving to phabricator differential soon.

A couple things:

1. The session at the Developer Summit (WikiDev16) included discussing this issue and the general consensus was that this isn't really a blocker as it is (or at least, no one in the room voiced any concerns when it was brought up at the very beginning, see the notes: T114320#1914902

2. From Mukunda in another task about the current state of things:

In T117058#2097407, @mmodell wrote:

@ArielGlenn: really arc diff and arc patch are the only arcanist commands that are absolutely critical. arc diff is needed for the reason you mentioned: because currently the only alternative is pasting a diff into a web form. Phabricator doesn't yet support "push to create revision" functionality. From my understanding of upstream plans, I believe that they will eventually implement a way to create a diff by simply git push HEAD:some/virtual/ref just like gerrit does.

(NB: that's this not yet merged change upstream: https://secure.phabricator.com/D9599 )

arc patch D123 is the command you would run to apply someone else's patch to your repo.

Finally arc land is useful but if you are avoiding arcanist you can do the git wrangling manually (git merge or rebase, git commit --amend, git push)

Summary: This is dependent upon upstream, and it is not a priority for them.

[offtopic]

In T127#2101732, @Paladox wrote:

Can the priority be changed since we're moving to phabricator differential soon.

@Paladox: For future reference: Not without providing actual arguments. Just because we move to Differential does not mean all tasks should suddenly have a higher priority. It would change nothing in reality, as manpower is still the same. :) (Please do not reply to this comment to keep this task focused on its actual topic.)

In T127#2101732, @Paladox wrote:

Can the priority be changed since we're moving to phabricator differential soon.

For the record, are you unable to install arcanist on your machine? Or is there another reason you want to work via plain git? https://www.mediawiki.org/wiki/Phabricator/Arcanist#Installation

Well I can install it, its just I'm used to using git. I found using Arcarnist hard to use plus I doint think phabricator works on windows which means that arcanist is not tested on windows.

In T127#1094814, @Mattflaschen wrote:

In T127#1083262, @chasemp wrote:

In T127#1083121, @JanZerebecki wrote:

What about being able to git fetch a patch that is still in review. Is that already supported upstream and just not shown in the UI?

It is supported

But see T136: Pulling patches from Phabricator does not give consistent commit hashes (people might get used to this in practice, not sure).

if you change a diff, it's going to change the hash in gerrit too.

In T127#2109957, @greg wrote:

In T127#2101732, @Paladox wrote:

Can the priority be changed since we're moving to phabricator differential soon.

For the record, are you unable to install arcanist on your machine? Or is there another reason you want to work via plain git? https://www.mediawiki.org/wiki/Phabricator/Arcanist#Installation

Just to weigh in, I use plain git right now (no git review, no wrappers) and I pretty much want to keep it that way, which is why I'm so interested in having workarounds for arcanist commands.

In T127#2110066, @ArielGlenn wrote:

Just to weigh in, I use plain git right now (no git review, no wrappers) and I pretty much want to keep it that way, which is why I'm so interested in having workarounds for arcanist commands.

epriestley has written a detailed response about the status of 'pure git' workflows in the upstream task: https://secure.phabricator.com/T5000#58172

I just applied https://secure.phabricator.com/D9599 to the version of phabricator we are running in production. It applies cleanly (see b4ccc4e6ce2cf4b94dd) and we could probably push this out, however, I worry about maintenance costs and potential merge conflicts if upstream goes in a different direction.

Another option: we could have something like a jenkins job watch for branches under a certain namespace and automatically create diffs from there using arcanist.

In T127#2110517, @mmodell wrote:

I just applied https://secure.phabricator.com/D9599 to the version of phabricator we are running in production. It applies cleanly (see b4ccc4e6ce2cf4b94dd) and we could probably push this out, however, I worry about maintenance costs and potential merge conflicts if upstream goes in a different direction.

Without commitment from upstream that they will merge something close to that patch, I'd say it's not a good idea to maintain it locally.

It might be possible to implement it in a way that doesn't patch core code. Certainly it would be possible to have a dedicated labs instance or something like that which receives git pushes and transforms them into differential diffs. I don't want to put much effort into it if upstream is going a different direction and they don't seem to think it's a high priority.

Personally I like arcanist so I'm not really bothered by using that.

Yes I think for now doing what you suggested by Jenkins listens and creates the diff as a better idea until upstream allow us to use git officially without arc.

How would the links look like on git since phabricator they are created differently to gerrit.

In T127#2110517, @mmodell wrote:

I just applied https://secure.phabricator.com/D9599 to the version of phabricator we are running in production. It applies cleanly (see b4ccc4e6ce2cf4b94dd) and we could probably push this out, however, I worry about maintenance costs and potential merge conflicts if upstream goes in a different direction.

Does this work by using our normal got setup by using the same commands we use to do with gerrit.

We should setup a test repo on diffusion here where users can test:

In T127#2111213, @Paladox wrote:

How would the links look like on git since phabricator they are created differently to gerrit.

What do you mean links?

In T127#2111225, @mmodell wrote:

In T127#2111213, @Paladox wrote:

How would the links look like on git since phabricator they are created differently to gerrit.

What do you mean links?

Well when we create patches through git on Windows and we push it, it shows the link where the patch was created in wondering will this work on git if you doint have arc.

In T127#2111226, @Paladox wrote:

Well when we create patches through git on Windows and we push it, it shows the link where the patch was created in wondering will this work on git if you doint have arc.

git review is not part of git.

Hi I doint use git review since it's not supported on Windows and I use the normal git. Git for Windows.

In T127#2111226, @Paladox wrote:

Well when we create patches through git on Windows and we push it, it shows the link where the patch was created in wondering will this work on git if you doint have arc.

Currently you can't submit a diff without arcanist so no. And in some hypothetical future where you use differential without arc, I can't really be sure how it would work.

I don't know how I forgot about this:

bloomberg/phabricator-tools: arcyd

usage: arcyd [-h]
             {init,list-repos,add-phabricator,add-repohost,add-repo,rm-repo,start,stop,restart,reload,fsck,fetch}
             ...

Arcyd - daemon to watch git repos, create and land reviews automatically.

Intended to make it easy for large teams to start using Differential without
individual contributors needing to install and configure Arcanist.

Individual contributors are still free to use Arcanist if they wish, Arcyd
provides a zero-config layer over Git to get them started.

Arcyd does the following:

• watches for specially named branches and automatically creates revisions
• automatically updates revisions when the branch changes
• automatically lands revisions when they are approved

minimal user workflow:

$ git checkout feature/mywork
~ commit some work on the branch ~
$ git push origin feature/mywork:arcyd-review/mywork/master

• Arcyd see's the 'arcyd-review' branch and creates a review ..
• Reviewer accepts the change ..
• Arcyd squashes the 'arcyd-review' branch onto master and deletes it ..

@mmodell So how would the above work ^^ it says you doint need to install any thing but then how does it work. Does it mean you doint need to install anything server side only on the pc your on making the patches. Also does it work on windows.

The user doesn't need to install anything (that's the point), but the Phab maintainers (ie: RelEng) would need to install that (arcyd) on our phab server. That adds a maintenance cost for us that we can review to see if it's worthwhile. :)

In T127#2159190, @greg wrote:

The user doesn't need to install anything (that's the point), but the Phab maintainers (ie: RelEng) would need to install that (arcyd) on our phab server. That adds a maintenance cost for us that we can review to see if it's worthwhile. :)

Oh ok, thanks for explaning I hope it is not too much of maintainance cost since if it allows us to use our normal install so git add -A --all then git commit then git push origin HEAD:refs/for/master then that seems like a good thin.

In T127#2109998, @mmodell wrote:

if you change a diff, it's going to change the hash in gerrit too.

Clearly. That task is about when you don't change the diff. My understanding is that Differential is diff-based, and last time I checked it does not have an equivalent to git review -d where the downloader always gets the same git hash.

@mattflaschen - with the use of arcanist and a staging repository it's possible to reference the same git commit id. With a staging repo configured, arc diff pushes a real git commit + a tag corresponding to the diff id, to whichever repository is specified in phabricator. Staging could be the same git repo or a separate one, depending on how it's set up.

In T127#2163993, @mmodell wrote:

@mattflaschen - with the use of arcanist and a staging repository it's possible to reference the same git commit id. With a staging repo configured, arc diff pushes a real git commit + a tag corresponding to the diff id, to whichever repository is specified in phabricator. Staging could be the same git repo or a separate one, depending on how it's set up.

Thanks for explaining.

@mmodell hi question: would it be possible to have arc installed on the server and when someone uses there normal git install and push would it be able to on the server create the the diff. But also a problem with the commit above that describes another tool that runs on the server is how do we upload new patches to that commit since I presume it won't use a change-Id instead it will be using the checkout id . So I'm wondering how can we check it out and amend it and upload it again.

Alsoit seems that repos are going to start being migrated to diffusion. See https://phabricator.wikimedia.org/T130418

All the migration is meant to happend in phase 3 or the remainder which will happend sometime after September and finish in December.

Maybe we can create a very good detailed guide on what commands we use in git like

git add -A --all

git commit

Amending a commit

git commit -a --amend

and git push origin HEAD:refs/for/master.

And possibly the git commands for the the first phase of the migration.

Maybe a new guide on how to install on Windows and Linux.

Please.

Maybe it would be easiest for phacility to add the ability to edit from the web browser to start with which would benefit everyone.

In T127#2166822, @Paladox wrote:

Alsoit seems that repos are going to start being migrated to diffusion. See https://phabricator.wikimedia.org/T130418

See the full context at https://www.mediawiki.org/wiki/Wikimedia_Release_Engineering_Team/Project/Differential_Migration

Maybe we can create a very good detailed guide on what commands we use in git like

Of course, that has always been the plan (with this task as well). See also T117058: Initial documentation of example Differential workflows (with Gerrit equivalents) and the other tasks in the "Documentation" column at Gerrit-Migration

Maybe a new guide on how to install on Windows and Linux.

For how to install arcanist see: https://www.mediawiki.org/wiki/Phabricator/Arcanist

For plain git (for eg Windows), installing arcanist won't an issue (by definition) and that is what this task is about.

Let's please keep this task on topic (interacting with Differential with git commands).

@greg and @mmodell we can use phabricator diff on the web tool.

See https://secure.phabricator.com/differential/diff/create/

We should enable it on phabricator here please.

I used my same git install. the only thing I had to change was not to use git push but to do git show > patch.txt

and upload that to the tool and create my patch.

So this is what I did

git add -A --all

git commit

even though you have to do your commit on the web, the web diff just pulls the diff not the comment

then I did

git show > patch.txt

then uploaded it to that tool

It was really easy and better then arc.

In T127#2169997, @Paladox wrote:

@greg and @mmodell we can use phabricator diff on the web tool.

See https://secure.phabricator.com/differential/diff/create/

We should enable it on phabricator here please.

That's always been available: https://phabricator.wikimedia.org/differential/diff/create/

But, that's not what this task is about and is just a work around that was mentioned very early in the comments on this task (T127#1791).

I'm hopeful for a solution like arcyd: T127#2159163.

We could get upstream to merge https://secure.phabricator.com/D9599 which is authored by @mmodell I tested it and it works instead of using arc you do git push origin HEAD:review which then uploads it to differential for reviewing but you go to the link and click create revision.

Differential supports limited landing from differential only if you have automation setup and a few other things.

The only that would need to be done is allowing users to amend the patch without arc.

So it should still say this

Apply Patch arc patch D2

but should add an additional field for pulling with git.

@mmodell would you be able to update your diff at https://secure.phabricator.com/D9599 with the change you described in your comment on your diff please. This works. This is one step towards not needing to use arc but not fully since you carn't amend patches without using arc.

@Paladox: it looks like the best route to differential without arcanist is using arcyd and I've broken that out into a separate task blocking this one. T132863: Set up arcyd to create differential revisions with `git push` (code review without arcanist)

Thanks.

Upstream have move https://secure.phabricator.com/T5000 to prioritised only to do the research and then eventually prioritised it.

From @faidon in wikitech-l:

If we have spare budget for the FY, a good start, I think, would be (properly) implementing https://secure.phabricator.com/T5000, by implementing https://secure.phabricator.com/T8092 which in turn depends on https://secure.phabricator.com/T8093 and possibly depends on https://secure.phabricator.com/T4369 and https://secure.phabricator.com/T4245.

In T127#2304394, @Qgil wrote:

From @faidon in wikitech-l:

If we have spare budget for the FY, a good start, I think, would be (properly) implementing https://secure.phabricator.com/T5000, by implementing https://secure.phabricator.com/T8092 which in turn depends on https://secure.phabricator.com/T8093 and possibly depends on https://secure.phabricator.com/T4369 and https://secure.phabricator.com/T4245.

In T127#2212556, @mmodell wrote:

@Paladox: it looks like the best route to differential without arcanist is using arcyd and I've broken that out into a separate task blocking this one. T132863: Set up arcyd to create differential revisions with `git push` (code review without arcanist)

Arcyd seems like a reasonable approach that has many parallels with the Gerrit model of pushing to special branches to create commits for review which anyone who doesn't currently use git-review would be used to doing.

@bd808 yep but one problem. if you accept a diff it gets merged automatically.

In T127#2305611, @Paladox wrote:

@bd808 yep but one problem. if you accept a diff it gets merged automatically.

Why would the review process via arcyd be different from the review process for patches submitted using another method (arc / upload)?

Because you have to have permission to merge whereas arcyd would merge for you. It doesn't check if the user that accepted can merge. Maybe they should add a button that you press to merge instead of it merging because you accepted.

In T127#2305629, @Paladox wrote:

Because you have to have permission to merge whereas arcyd would merge for you. It doesn't check if the user that accepted can merge. Maybe they should add a button that you press to merge instead of it merging because you accepted.

Phabricator already has a built in 'merge automation' and that does support access policies. I don't think we would have arcyd handle merging. I'd suggest that someone use arc to merge manually or we set up phabricator merge automation.

@mmodell would we be able to setup phabricator merge automation please. Would make it easy for mobile users to merge patches they have reviewed please.

@Paladox: I plan to experiment with this soon, I just haven't had enough time to spend on it yet. It's on the radar.

@mmodell Thanks.

Only slightly related in terms of convenience and not "solving" this task, Collabora has been maintaining a tool called git-phab.
See https://phabricator.freedesktop.org/diffusion/GITPHAB/#workflow-example

Upstream has indicated that they will be working on https://secure.phabricator.com/T5000 soon.

Upstream have opened a new task https://secure.phabricator.com/T12010

And looks like they have started working on this.

Would https://phabricator.freedesktop.org/diffusion/GITPHAB/ work? idk how I missed it being mentioned above