diff mbox

gnu: Add git-review.

Message ID 20160908183217.GK12493@macbook42.flashner.co.il
State New
Headers show

Commit Message

Efraim Flashner Sept. 8, 2016, 6:32 p.m. UTC 
On Thu, Sep 08, 2016 at 10:28:37PM +1000, Ben Woodcroft wrote:
> Hi there, thanks for the patch. I do not have any experience with Gerrit but
> some comments below:
> 
> 
> On 08/09/16 17:06, Clément Lassieur wrote:
> > [..]
> > +    (source
> > +     (origin
> > +       (method url-fetch)
> > +       (uri (string-append
> > +             "https://pypi.python.org/packages/source/g/git-review/git-review-"
> > +             version ".tar.gz"))
> 
> Rather: (uri (pypi-uri "git-review" version))
> 
> > +       (sha256
> > +        (base32
> > +         "07d1jn9ryff5j5ic6qj5pbk10m1ccmpllj0wyalrcms1q9yhlzh8"))))
> > +    (build-system python-build-system)
> > +    (arguments `(#:tests? #f)) ; tests require a running Gerrit server
> > +    (native-inputs
> > +     `(("python-pbr" ,python-pbr)))
> > +    (inputs
> > +     `(("python-requests" ,python-requests)
> > +       ("git" ,git)))
> 
> 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

Comments

Clément Lassieur Sept. 9, 2016, 8:23 a.m. UTC | #1 
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.
diff mbox

Patch

diff --git a/gnu/packages/openstack.scm b/gnu/packages/openstack.scm
index 4cb38a9..d7876a8 100644
--- a/gnu/packages/openstack.scm
+++ b/gnu/packages/openstack.scm
@@ -796,7 +796,19 @@  permanence.")
         (base32
          "07d1jn9ryff5j5ic6qj5pbk10m1ccmpllj0wyalrcms1q9yhlzh8"))))
     (build-system python-build-system)
-    (arguments `(#:tests? #f)) ; tests require a running Gerrit server
+    (arguments
+     `(#:tests? #f ; tests require a running Gerrit server
+       #:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'patch-git-calls
+           (lambda _
+             (let ((git (string-append
+                          (assoc-ref %build-inputs "git") "/bin/git")))
+               (substitute* "git_review/cmd.py"
+                            (("\"git ") (string-append "\"" git " "))
+                            (("\"git\"") (string-append "\"" git "\""))
+                            (("'git'") (string-append "'" git "'")))
+               #t))))))
     (native-inputs
      `(("python-pbr" ,python-pbr)))
     (inputs