janitor icon indicating copy to clipboard operation
janitor copied to clipboard

Firefox Git not properly setup for contributing

Open rugk opened this issue 6 years ago • 15 comments

When running arc diff in the Firefox git, I get:

Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 Exception 
ERR-CONDUIT-CORE: Local VCS (git) is different from the one defined in the repository (hg).
To use Phabricator with git-cinnabar please follow the docs at https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-git-cinnabar
(Run with `--trace` for a full exception trace.)

Maybe the wrong (unpatched) arc version is installed?

It is very bad to discover that, after you've made your patch ready and are almost finished. :cry:

rugk avatar Jan 24 '19 21:01 rugk

@rugk I am sorry to hear this, thank you very much for taking the time to tell us about this issue. I am not a user of arc but I ran arc --version and these are my results:

arcanist 3534d2baca4bf6dcdac46c49164bf5ba3a6660ad (8 Nov 2018) libphutil 35d0ec2dfa595fd77410b67032e3e4262d170b6d (21 Nov 2018)

Do you know if we need to update arc based on the above?

Coder206 avatar Jan 24 '19 21:01 Coder206

I just follow the official guide and so it recommends arc. Don't even know what I should use otherwise.

And no, I have no idea… :laughing: It should be the build from https://github.com/mozilla-conduit/arcanist certainly…

rugk avatar Jan 24 '19 21:01 rugk

@rugk Looking over the last commit message (a few days after the one I shared above), looks like it addresses an issue related to arc diff with large files.

A temporary solution could be to try to compile it in your own container from the instructions, I think these are the ones https://secure.phabricator.com/book/phabricator/article/arcanist/#installing-arcanist.

Coder206 avatar Jan 24 '19 21:01 Coder206

Yeah, now cloned the latest patched version:

$ arc --version
arcanist 93f869a7c6c3b6c3e88d1d9882af14c9177a4ecc (13 Nov 2018)
libphutil 1d3b33d4ccbf776132463e69d1a10f0e07020363 (21 Jan 2019)

Nevertheless that should obviously already be provided in the container – I mean that's the whole sense of it, so you do not need to set it up by yourself.

rugk avatar Jan 24 '19 21:01 rugk

@rugk Please let us know if that rectifies the issue.

Coder206 avatar Jan 24 '19 21:01 Coder206

Okay, strange, I get the same issue. But I certainly cloned the patched version.

With --trace now:

>> [3] (+4,339) <event> diff.didCollectChanges <listeners = 0>
<<< [3] (+4,340) <event> 184 us
>>> [4] (+4,340) <exec> $ git rev-parse --verify HEAD^
<<< [4] (+4,347) <exec> 7,097 us
>>> [5] (+4,348) <exec> $ git rev-parse --abbrev-ref --symbolic-full-name '@{upstream}'
<<< [5] (+4,354) <exec> 6,646 us
>>> [6] (+4,355) <exec> $ git cat-file -t 'origin/master'
<<< [6] (+4,361) <exec> 6,204 us
>>> [7] (+4,361) <exec> $ git merge-base 'origin/master' HEAD
<<< [7] (+4,367) <exec> 6,190 us
>>> [8] (+4,368) <exec> $ git rev-parse 'HEAD'
<<< [8] (+4,372) <exec> 4,882 us
>>> [9] (+4,373) <exec> $ git log --first-parent --format=medium '3564dfde3b125878dc5a04fe92629fc5195942df'..'00b2a21dccc618c26513e6419078dd677cecfe67'
<<< [9] (+4,380) <exec> 7,277 us
>>> [10] (+4,386) <exec> $ git log '00b2a21dccc618c26513e6419078dd677cecfe67' --not '3564dfde3b125878dc5a04fe92629fc5195942df' --format='%H%x01%T%x01%P%x01%at%x01%an%x01%aE%x01%s%x01%s%n%n%b%x02' --
<<< [10] (+4,394) <exec> 7,859 us
>>> [11] (+4,394) <http> https://phabricator.services.mozilla.com/api/differential.query
<<< [11] (+4,654) <http> 259,954 us
>>> [12] (+4,654) <exec> $ git rev-parse --git-dir
<<< [12] (+4,661) <exec> 6,689 us
>>> [13] (+4,662) <exec> $ git log '00b2a21dccc618c26513e6419078dd677cecfe67' --not '3564dfde3b125878dc5a04fe92629fc5195942df' --format='%H%x01%T%x01%P%x01%at%x01%an%x01%aE%x01%s%x01%s%n%n%b%x02' --
<<< [13] (+4,670) <exec> 8,455 us
>>> [14] (+4,670) <http> https://phabricator.services.mozilla.com/api/differential.parsecommitmessage
<<< [14] (+4,931) <http> 260,179 us
>>> [15] (+4,931) <event> diff.willBuildMessage <listeners = 0>
<<< [15] (+4,931) <event> 144 us
>>> [16] (+4,932) <http> https://phabricator.services.mozilla.com/api/differential.getcommitmessage
<<< [16] (+5,192) <http> 260,177 us
>>> [17] (+5,192) <exec> $ git symbolic-ref --quiet HEAD
<<< [17] (+5,199) <exec> 6,712 us
>>> [18] (+5,202) <exec> $ nano '/tmp/edit.b76s1b2m4bkk4gow/new-commit'
<<< [18] (+6,315) <exec> 1,113,439 us
>>> [19] (+6,317) <http> https://phabricator.services.mozilla.com/api/differential.parsecommitmessage
<<< [19] (+6,696) <http> 379,165 us
>>> [20] (+6,698) <conduit> user.query() <bytes = 215>
>>> [21] (+6,698) <http> https://phabricator.services.mozilla.com/api/user.query
<<< [21] (+7,007) <http> 308,743 us
<<< [20] (+7,007) <conduit> 309,147 us
>>> [22] (+7,007) <event> diff.didBuildMessage <listeners = 0>
<<< [22] (+7,007) <event> 86 us
Linting...
No lint engine configured for this project.
Running unit tests...
>>> [23] (+7,010) <exec> $ git rev-parse 'HEAD'
<<< [23] (+7,017) <exec> 6,768 us
>>> [24] (+7,017) <exec> $ git merge-base '3564dfde3b125878dc5a04fe92629fc5195942df' '00b2a21dccc618c26513e6419078dd677cecfe67'
<<< [24] (+7,025) <exec> 7,268 us
>>> [25] (+7,025) <exec> $ git diff --no-ext-diff --no-textconv --submodule=short --raw '3564dfde3b125878dc5a04fe92629fc5195942df' HEAD --
<<< [25] (+7,033) <exec> 8,228 us
No unit test engine is configured for this project.
>>> [26] (+7,034) <exec> $ git diff --no-ext-diff --no-textconv --submodule=short --no-color --src-prefix=a/ --dst-prefix=b/ -U32767 -M -C '3564dfde3b125878dc5a04fe92629fc5195942df' '00b2a21dccc618c26513e6419078dd677cecfe67' --
<<< [26] (+7,042) <exec> 8,571 us
>>> [27] (+7,045) <exec> $ git rev-parse '3564dfde3b125878dc5a04fe92629fc5195942df'
<<< [27] (+7,051) <exec> 5,460 us
>>> [28] (+7,051) <exec> $ git cinnabar git2hg '3564dfde3b125878dc5a04fe92629fc5195942df'
<<< [28] (+7,260) <exec> 208,327 us
>>> [29] (+7,260) <exec> $ git rev-parse --revs-only refs/cinnabar/metadata
<<< [29] (+7,264) <exec> 4,625 us
>>> [30] (+7,265) <exec> $ git symbolic-ref --quiet HEAD
<<< [30] (+7,269) <exec> 4,725 us
>>> [31] (+7,270) <exec> $ git log --format=medium -n16 '3564dfde3b125878dc5a04fe92629fc5195942df'
<<< [31] (+7,277) <exec> 7,765 us
>>> [32] (+7,279) <http> https://phabricator.services.mozilla.com/api/repository.query
<<< [32] (+7,544) <http> 264,889 us
>>> [33] (+7,545) <http> https://phabricator.services.mozilla.com/api/differential.creatediff
<<< [33] (+8,109) <http> 563,872 us

[2019-01-24 21:51:02] EXCEPTION: (ConduitClientException) ERR-CONDUIT-CORE: Local VCS (git) is different from the one defined in the repository (hg).
To use Phabricator with git-cinnabar please follow the docs at https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-git-cinnabar at [<phutil>/src/conduit/ConduitFuture.php:62]
arcanist(head=stable, ref.stable=93f869a7c6c3), phutil(head=master, ref.master=1d3b33d4ccbf)
  #0 ConduitFuture::didReceiveResult(array) called at [<phutil>/src/future/FutureProxy.php:58]
  #1 FutureProxy::getResult() called at [<phutil>/src/future/FutureProxy.php:35]
  #2 FutureProxy::resolve() called at [<phutil>/src/conduit/ConduitClient.php:64]
  #3 ConduitClient::callMethodSynchronous(string, array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:519]
  #4 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:394]

rugk avatar Jan 24 '19 21:01 rugk

@rugk Did you follow the instructions for setting it up for git: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html#using-git-cinnabar ?

Coder206 avatar Jan 24 '19 21:01 Coder206

Ugh, so this git is also not setup?

At your link there are only instructions for arc

rugk avatar Jan 24 '19 21:01 rugk

@rugk git is a tool to keep track of changes in files, which is already installed. Looks like arc needs to be configured to use git instead of the alternative hg (Mercurial).

Coder206 avatar Jan 24 '19 21:01 Coder206

Yeah, I am at it… docs are at https://github.com/glandium/git-cinnabar#setup

rugk avatar Jan 24 '19 21:01 rugk

$ git cinnabar git2hg 00b2a21dccc618c26513e6419078dd677cecfe67
0000000000000000000000000000000000000000

:thinking:

rugk avatar Jan 24 '19 22:01 rugk

Finally just created a new hg container to submit these changes, but yeah… the git container is definitively not completely setup… But BTW, thanks for your help, @Coder206 .

rugk avatar Jan 24 '19 22:01 rugk

Thanks @Coder206 for looking into this! And sorry @rugk for the bad experience.

In fact, it's simply arc arbitrarily refusing to work because it sees a Git repository, but you can "convince" it to work by hacking it like so (in /home/user/.phacility/arcanist/):

diff --git a/src/repository/api/ArcanistGitAPI.php b/src/repository/api/ArcanistGitAPI.php
index c7149bd1..b901ff4c 100644
--- a/src/repository/api/ArcanistGitAPI.php
+++ b/src/repository/api/ArcanistGitAPI.php
@@ -48,7 +48,7 @@ public function execPassthru($pattern /* , ... */) {
 
 
   public function getSourceControlSystemName() {
-    return 'git';
+    return 'hg';
   }
 
   public function getGitVersion() {

P.S. Sorry for the very bad experience with arc diff. The preferred way to send patches from the "Firefox (git)" image is to use phlay form the regular Terminal.

jankeromnes avatar Jan 25 '19 13:01 jankeromnes

The preferred way to send patches from the "Firefox (git)" image is to use phlay form the regular Terminal.

That's not really documented anywhere (at least the starting docs I've looked at and linked here several times already), and the recommend way now seems to be moz-phab, but see https://github.com/JanitorTechnology/janitor/issues/422 about it. So yeah, it's a little "a lot" for a new contributor to start and first to find the right tool… :wink:

rugk avatar Jan 26 '19 01:01 rugk

So yeah, it's a little "a lot" for a new contributor to start and first to find the right tool… 😉

I agree, and I'm sorry about the confusing contribution experience! We should really aim to make things as simple and streamlined as possible.

For what it's worth, phlay is a really small script, that works extremely well, with any Git clone of Firefox (even if it's not git-cinnabar-based).

I'm not quite sure why Mozilla decided to also build Git-support into moz-phab, which is slower and doesn't work as well (and it requires git-cinnabar, which is also quite complicated to set up). But they've decided that moz-phab should be the official tool, so that's what the docs mention today.

Let's have both available in Janitor containers. 😄 That seems to be a good compromise.

jankeromnes avatar Jan 28 '19 10:01 jankeromnes