gnu: Add git-review.

On Fri, Sep 09, 2016 at 10:23:20AM +0200, Clément Lassieur wrote:
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > On Thu, Sep 08, 2016 at 10:28:37PM +1000, Ben Woodcroft wrote:
> 
> >> I think including git simply as an input is problematic because git-review
> >> calls git via 'subprocess', as evidenced by
> >> 
> >> $ ./pre-inst-env guix environment -C --ad-hoc git-review
> >> $ git-review
> >> [..]
> >>   File "/gnu/store/m4gc2wx4q9if1vrhgclpspdil7rqsn21-python-3.4.3/lib/python3.4/subprocess.py",
> >> line 1457, in _execute_child
> >>     raise child_exception_type(errno_num, err_msg)
> >> FileNotFoundError: [Errno 2] No such file or directory: 'git'
> >> 
> >> So, I think we need to patch the source code to call the full path to git,
> >> or otherwise wrap the 'git-review' executable.
> >> 
> >> > +    (home-page "http://docs.openstack.org/infra/git-review/")
> >> > +    (synopsis "Command-line tool for Gerrit")
> >> > +    (description
> >> > +     "Git-review is a command-line tool that helps submitting Git branches to
> >> > +Gerrit for review, or fetching existing ones.")
> >> > +    (license asl2.0)))
> >> Otherwise seems OK to me. Can you test with environment -C -N?
> >> ben
> >> 
> >
> > Try it with the attached patch
> 
> Hi, sorry for the mess and thanks for the reviews.
> 
> I did not know about the "guix environment -C -N" way of testing
> packages, and I'm very happy to learn this.
> 
> The attached patch seems to work for git, but the same error now happens
> with ssh.
> 
> -- 
> Clément

I found ssh and I found an instance of scp, so this should be better.

Efraim Flashner <efraim@flashner.co.il> writes:

> I found ssh and I found an instance of scp, so this should be better.

--8<---------------cut here---------------start------------->8---
clement@lev ~/foo [env]# git-review -d 36373
Downloading refs/changes/73/36373/1 from gerrit
Cannot fetch patchset contents

Does specified change number belong to this project?

The following command failed with exit code 128
    "/gnu/store/...-git-2.10.0/bin/git fetch gerrit refs/changes/73/36373/1"
-----------------------
error: cannot run ssh: No such file or directory
fatal: unable to fork
-----------------------
clement@lev ~/foo [env]# /gnu/store/...-git-2.10.0/bin/git remote -v                           
gerrit  ssh://bar (fetch)
gerrit  ssh://bar (push)
--8<---------------cut here---------------end--------------->8---

It looks like ssh isn't hardcoded within git either. Is it worth doing
this for git-review then?

Efraim Flashner (2016-09-09 18:00 +0300) wrote:

[...]
> +    (arguments
> +     `(#:tests? #f ; tests require a running Gerrit server
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'hardcode-outside-programs
> +           (lambda _

A side note: I think in phases it is better to use 'inputs' argument
instead of %build-inputs:

              (lambda* (#:key inputs #:allow-other-keys)

> +             (let* ((git  (string-append
> +                            (assoc-ref %build-inputs "git") "/bin/git"))
> +                    (base (assoc-ref %build-inputs "openssh"))

(assoc-ref inputs "git") and (assoc-ref inputs "openssh") here

> +                    (ssh  (string-append base "/bin/ssh"))
> +                    (scp  (string-append base "/bin/scp")))
> +               (substitute* '("git_review/cmd.py" "git_review/tests/test_git_review.py"
> +                              "git_review/tests/test_unit.py" "git_review/tests/utils.py")
> +                            (("\"git ") (string-append "\"" git " "))
> +                            (("\"git\"") (string-append "\"" git "\""))
> +                            (("'git'") (string-append "'" git "'")))
> +               (substitute* "git_review/cmd.py"
> +                            (("\"ssh\"") (string-append "\"" ssh "\""))
> +                            (("'ssh'") (string-append "'" ssh "'"))
> +                            (("\"scp\"") (string-append "\"" scp "\""))
> +                            (("'scp") (string-append "'" scp)))
> +               #t))))))
>      (native-inputs
>       `(("python-pbr" ,python-pbr)))
>      (inputs
>       `(("python-requests" ,python-requests)
> +       ("openssh" ,openssh)
>         ("git" ,git)))
>      (home-page "http://docs.openstack.org/infra/git-review/")
>      (synopsis "Command-line tool for Gerrit")

On Fri, Sep 09, 2016 at 06:09:40PM +0200, Clément Lassieur wrote:
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > I found ssh and I found an instance of scp, so this should be better.
> 
> --8<---------------cut here---------------start------------->8---
> clement@lev ~/foo [env]# git-review -d 36373
> Downloading refs/changes/73/36373/1 from gerrit
> Cannot fetch patchset contents
> 
> Does specified change number belong to this project?
> 
> The following command failed with exit code 128
>     "/gnu/store/...-git-2.10.0/bin/git fetch gerrit refs/changes/73/36373/1"
> -----------------------
> error: cannot run ssh: No such file or directory
> fatal: unable to fork
> -----------------------
> clement@lev ~/foo [env]# /gnu/store/...-git-2.10.0/bin/git remote -v                           
> gerrit  ssh://bar (fetch)
> gerrit  ssh://bar (push)
> --8<---------------cut here---------------end--------------->8---
> 
> It looks like ssh isn't hardcoded within git either. Is it worth doing
> this for git-review then?

This looks like you are using ssh for talking to the remote server, in
which case it would exist outside of git-review. Or did I misunderstand
it?

It does seem that wrapping the program would be easier than trying to
fing regexes that work to patching only certain instances of other
programs.

Efraim Flashner <efraim@flashner.co.il> writes:

>> It looks like ssh isn't hardcoded within git either. Is it worth doing
>> this for git-review then?
>
> This looks like you are using ssh for talking to the remote server, in
> which case it would exist outside of git-review. Or did I misunderstand
> it?

This error was a git error, when it tries to fetch a remote through ssh
in a container. It is kind of the same thing that happens with
git-review. Except that git-review is unusable without ssh. That's why
ssh has to be an input of git-review.

> It does seem that wrapping the program would be easier than trying to
> fing regexes that work to patching only certain instances of other
> programs.

I sent a patch that does the wrapping.