From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48420) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1g0RAF-0004sC-9N for guix-patches@gnu.org; Thu, 13 Sep 2018 08:56:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1g0RAA-00054I-4y for guix-patches@gnu.org; Thu, 13 Sep 2018 08:56:07 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:34560) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1g0RA9-000543-QJ for guix-patches@gnu.org; Thu, 13 Sep 2018 08:56:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1g0RA9-0002V1-NN for guix-patches@gnu.org; Thu, 13 Sep 2018 08:56:01 -0400 Subject: [bug#32699] Adding r-gtk2 and other packages for r-qda (RFC) References: <20180911122320.eyg4rms4eh36dnzx@thebird.nl> In-Reply-To: <20180911122320.eyg4rms4eh36dnzx@thebird.nl> Resent-Message-ID: From: Ricardo Wurmus Date: Thu, 13 Sep 2018 14:55:31 +0200 Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: 32699@debbugs.gnu.org Hi Pjotr, thanks for the patches! I=E2=80=99m glad that you found the recursive importer helpful. Unfortunat= ely, it doesn=E2=80=99t do a great job with licenses: some of the packages have a list of GPLv2+ and GPLv3+. Sometimes this is really just a CRAN expansion for =E2=80=9CGPLv2 or later=E2=80=9D, other times it means that d= ifferent parts of the package are under different licenses. Could you please check what the license list means for r-rgtk2 and r-cairodevice? In some cases we can replace the list with a single license; in other cases we add a comment above the license field to state what it really means. Generally, I would recommend putting these packages in (gnu packages cran) instead of (gnu packages statistics). I=E2=80=99ve been moving packa= ges from statistics.scm to cran.scm to avoid circular dependencies between modules. Here some more comments and questions about the patches: * the importer unfortunately still defaults to generating HTTP URLs for the home page instead of HTTPS. It also fails to add a trailing slash for cran.r-project.org URLs. Could you please add them? (This keeps the linter happy.) * the description of many R packages on CRAN don=E2=80=99t use full sentenc= es, whereas for Guix we=E2=80=99d like to use complete sentences. The easies= t way to fix this is to add =E2=80=9CThis package provides=E2=80=9D to the begi= nning of the package description from CRAN. (The importer does this automatically, when the description begins with =E2=80=9CA =E2=80=9D, but it can=E2=80= =99t easily guess other instances where this would be appropriate.) * the synopses of R packages from CRAN often use =E2=80=A6 =E2=80=9Ccreativ= e=E2=80=9D letter casing. Instead of, say, =E2=80=9CR Bindings for Gtk 2.8.0 and Above=E2= =80=9D we would use =E2=80=9CR bindings for Gtk 2.8.0 and above=E2=80=9D. (I don= =E2=80=99t know how to let the importer do the right thing automatically.) * for r-cairodevice we might be able to run the tests after setting up an X server in a pre-check build phase. We would need to add ("xorg-server" ,xorg-server) to the native inputs. Here=E2=80=99s an exa= mple for the phase taken from the caja package: --8<---------------cut here---------------start------------->8--- #:phases (modify-phases %standard-phases (add-before 'check 'pre-check (lambda _ ;; Tests require a running X server. (system "Xvfb :1 &") (setenv "DISPLAY" ":1") ;; For the missing /etc/machine-id. (setenv "DBUS_FATAL_WARNINGS" "0") #t))) --8<---------------cut here---------------end--------------->8--- * the description in r-cairodevice includes references to code: we would, for example, replace =E2=80=9Carbitrary GdkDrawable or Cairo conte= xt=E2=80=9D with =E2=80=9Carbitrary @code{GdkDrawable} or @code{Cairo} context=E2=80= =9D. Same with =E2=80=9CRGtk2=E2=80=9D and =E2=80=9CgetGraphicsEvent()=E2=80=9D. * I would replace the description of =E2=80=9Cr-gwidgets=E2=80=9D with this= text: "gWidgets provides a toolkit-independent API for building interactive GUIs. At least one of the toolkit implementations for a specific GUI backend (such as @code{gWidgetsRGtk2}) should be installed." * In r-gwidgetsrgtk2 we might be able to do without patching the DESCRIPTION and NAMESPACE files by adding a pre-check phase as shown above. Is there another reason for patching this file? * I would change the r-gwidgetsrgtk2 description to this (taken from the package documentation): =E2=80=9CThis package allows the gWidgets API to use the RGtk2 package allowing the use of the GTK libraries within R.=E2=80=9D * r-igraph already exists in (gnu packages graph). * r-rqda uses sources from a git checkout, which means that you could use git-specific helpers: (version (git-version "0.3-1" revision commit)) =E2=80=A6 (file-name (git-file-name name version)) Note that the revision is just a Guix-internal number (as a string) starting at "1". * r-nlp: the same applies as for other packages: please add it to cran.scm, use HTTPS and a trailing slash for the home-page field, use lower case for the synopsis, and expand the description to a complete sentence. * r-tm looks fine to me. To generate patches more easily you can create one local commit per package and then use git format-patch -5 to create 5 patch files for the last 5 commits. We usually apply one patch per package. -- Ricardo