GNU bug report logs - #40426
[PATCH] Add g-golf

Previous Next

Full log

View this message in rfc822 format

From: Tobias Geerinckx-Rice <me <at> tobias.gr>
To: Vitaliy Shatrov <D0dyBo0D0dyBo0 <at> protonmail.com>
Cc: 40426 <at> debbugs.gnu.org
Subject: [bug#40426] [PATCH] Add g-golf
Date: Wed, 15 Apr 2020 15:57:03 +0200

[Message part 1 (text/plain, inline)]

Vitaliy,

Vitaliy Shatrov via Guix-patches via 写道：
> str1ngs and nly want it to be submitted to guix, and i was 
> proudly take this task.  Package was copied "as-is", and tested 
> as per Guix manual.

Thank you!

> There is desire to package be named "g-golf", and not as 
> "guile-g-golf", as the package name stands for "Gnome: Guile 
> Object Library For".

Too clever for me :-)  It's a Guile library; hence the correct 
Guix name (and variable) is ‘guile-g-golf’.  We have plenty of 
‘python-pyfoo’ packages to keep it company.

> Subject: [PATCH] gnu: Add g-golf
>
> * gnu/packages/guile-xyz.scm (g-golf): New variable

Nitpick: both lines should end with a full stop.

> +(define-public g-golf

Could you add a comment here explaining why we use a git commit, 
instead of a release tarball or tag?  I assume there are none; 
that would do as comment.  However…

> +  (let ((commit "4a4edf25e4877df9182c77843bdd98ab59e13ef7"))
> +    (package
> +      (name "g-golf")
> +      (version (git-version "1" "683" commit))

…‘1’ means the project has released version 1 prior to this 
commit, or at least regards this commit as part of the ‘1’ series. 
I didn't spot any version number on the home page, NEWS file, git 
tags, …

If there is no ‘1’ release, use ‘0.0.0’.

The second field (REVISION) should be ‘0’, since this is the first 
*Guix* revision of this package.  The idea is that you increment 
the revision each time you change COMMIT, so Guix knows which 
commit is newer and can ‘guix package -u’ properly.

Since the 2 should be updated together, bind them together:

 (let ((commit "f00")
       (revision "0")) …

You obviously got ‘683’ from somewhere though.  Where?

> +       `(#:tests? #t

Does the guile-build-system disable tests by default?  (I skimmed 
the code but didn't find anything.)  Otherwise, this can be 
omitted.

> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-before 'configure 'tests-work-arounds

Prefer ‘verb-thing’; makes it much easier to skim unfamiliar 
packages.

In this case we're not really working around the tests themselves, 
so I'd go with the boring ‘patch-tests’.

+             (lambda* (#:key inputs #:allow-other-keys)
+               ;; In build environment, There is no /dev/tty
+               (substitute*
+                   "test-suite/tests/gobject.scm"
+                 (("/dev/tty") "/dev/null"))))

For now, all phases must return #t.  SUBSTITUTE* doesn't, so we 
need

            (lambda …
              …
              (substitute* "test-suite/tests/gobject.scm"
                (("/dev/tty") "/dev/null"))
              #t)

No need to put the file name on its own line here.

+           (add-before 'configure 'substitute-libs

Bytes are cheap: ‘libraries’.

Kind regards,

T G-R

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.