Access remote git repositories (ssh scheme url) without a raw file url.

Review Request #4910 — Created Nov. 6, 2013 and discarded

Information

jop
Review Board
master

Reviewers

Access remote git repositories (ssh scheme url) without a raw file url.

Git-shell, as usually configured on ssh git servers, allows executing
arbitrary commands. This patch uses this feature to avoid having to setup
an http server for raw files.

To use this feature:

1. Set both path and mirror path to the same ssh git url, without the .git
sufix.

2. Leave raw file url mask empty.

3. Add the following script in ~/git-shell-commans/rb-cat-file:

------------
#!/usr/bin/env sh
git --git-dir="$1/.git" cat-file $2 $3
------------

This assumes that ssh authentication is working, by having setup the
proper ssh authorization at the server.

This has been tested in release 1.7.16 (current Fedora rpm installation) with a remote git repo on a different server. It works for submitting and updating review requests from rbt and the web interface.

Tests with invalid setups are as follows:

  1. Missing chsh to git-shell:


    $ rbt post -g
    ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: bash: rb-cat-file: command not found

Your review request still exists, but the diff is not attached.

  1. Not chmod-ing the script 755:


    $ rbt post -g
    ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'

Your review request still exists, but the diff is not attached.

  1. Not having the script at all:


    $ rbt post -g
    ERROR:root:Error uploading diff

One or more fields had errors (HTTP 400, API Error 105)

path: fatal: unrecognized command 'rb-cat-file /path/to/repo -t pha7AhquZa7icohBeoPh2AjuuthaiNg4faiMi9oo'

Your review request still exists, but the diff is not attached.

  1. Any of the above from the web interface: the same error (i.e. the "path:" line) appears in red by the diff selector.

  1. Corrupting the commit id fed to the script (this can only be done by changing the source code):


    $ rbt post -g
    ERROR:root:Error uploading diff

The file was not found in the repository (HTTP 400, API Error 207)

Your review request still exists, but the diff is not attached.

  1. Corrupting the commit id fed to the script in the web interface (this can only be done by changing the source code):

"The file 'README' (rd702173) could not be found in the repository" appears in red by the diff selector.

chipx86
  1. This is definitely an interesting approach, but how sure are you that all installs allow arbitrary commands?

    There's no way we can require placing a script on there. Can this not be done without a custom script?

    1. AFAIK this cannot be done without the script. This git limitation is actually the reason for having the "Raw file URL mask" workaround, right?
      
      But note that no requirement is actually being added. Every setup that has previously worked, either with an HTTP server or a local repo mirror, will work unchanged. That would still be the preferred setup with publicly available repos that already have cgit/gitweb access.
      
      This patch only adds another option. And when you have a typical private git repo with ssh access, it seems to me easier to add the script than either (1) setting up an http server with cgit/gitweb (including a different authentication mechanism...); or (2) having a local repository mirror. But it certainly depends on each setup.
      
      Arbitrary scripts are a standard feature of git-shell (http://git-scm.com/docs/git-shell.html), which is the recommended setup for remote ssh access to git repos.
    2. Sure. At this point, it's more that we have yet another method to document and support, but I can see the value in it. We'll have to discuss what we want to do here.
      
      Can you go through the change and make sure it's consistent with the rest of our code? Right now, it's missing things like spaces around operators, and I'd like to see it a bit closer before doing an initial review.
  2. 
      
JO
chipx86
  1. You seem to have renamed the summary.

    Can you also provide the Testing Done section? Main things I want to see:

    1. Successful tests with the script installed.
    2. Tests with invalid data passed to the script.
    3. The script not installed (or not installed correctly -- wrong chmod, for instance), and how that's handled.
  2. 
      
JO
JO
Review request changed
Status:
Discarded