diff mbox

gnu: Add gparted.

Message ID 87shx2xjrj.fsf@gnu.org
State New
Headers show

Commit Message

Roel Janssen May 28, 2016, 3:52 p.m. UTC
Dear Guix,

I wonder if my ChangeLog-format commit message is OK this way.
Also, is it OK to push this patch for GParted?  To avoid a name
collision for 'zlib' (both a license and a package), I added the
usual 'license:' prefix to the other packages in this file.

Thanks!

Kind regards,
Roel Janssen


>From d9ded46817468a6b63f648efbe9aa2a2dee59c93 Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Sat, 28 May 2016 17:42:22 +0200
Subject: [PATCH] gnu: Add gparted.

* gnu/packages/disk.scm (gparted): New variable.
(parted, fdisk, ddrescue, dosfstools, sdparm, idle3-tools): Avoid ambiguity by
adding a prefix to license symbols.
---
 gnu/packages/disk.scm | 63 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 55 insertions(+), 8 deletions(-)

Comments

Leo Famulari May 28, 2016, 4:18 p.m. UTC | #1
On Sat, May 28, 2016 at 05:52:16PM +0200, Roel Janssen wrote:
> Dear Guix,
> 
> I wonder if my ChangeLog-format commit message is OK this way.

How about wording it as in bf50f7b5870? I don't think it's necessary to
list the affected packages if it applies to the entire module.

> Also, is it OK to push this patch for GParted?  To avoid a name
> collision for 'zlib' (both a license and a package), I added the
> usual 'license:' prefix to the other packages in this file.

Okay!

> * gnu/packages/disk.scm (gparted): New variable.
> (parted, fdisk, ddrescue, dosfstools, sdparm, idle3-tools): Avoid ambiguity by
> adding a prefix to license symbols.

It runs and finds my partitions! I didn't try to actually do anything...

> +    (arguments
> +     `(#:tests? #f ; Tests require a network connection.

Is it just a small subset of the tests that require the network? Maybe
we could disable only those tests?

> +       #:configure-flags '("--disable-scrollkeeper")))

I'm not sure what this means. Can you leave a comment explaining it?

> +    (inputs
> +     `(("util-linux" ,util-linux)

Since util-linux is just a mish-mash of miscellaneous utilities, I like
it when packagers leave a hint of what it's being used for, either in a
comment, or by naming it after the used library, as in the jack-1
package.
Roel Janssen May 28, 2016, 6:34 p.m. UTC | #2
Hello Leo,

Thank you for your quick response.

Leo Famulari writes:

> On Sat, May 28, 2016 at 05:52:16PM +0200, Roel Janssen wrote:
>> Dear Guix,
>> 
>> I wonder if my ChangeLog-format commit message is OK this way.
>
> How about wording it as in bf50f7b5870? I don't think it's necessary to
> list the affected packages if it applies to the entire module.

Right.  I had looked for a similar change but could not find one.
Thanks for pointing this one out.

Would it perhaps be better if I split the license prefix changes and
the addition of gparted in two separate patches?

>> Also, is it OK to push this patch for GParted?  To avoid a name
>> collision for 'zlib' (both a license and a package), I added the
>> usual 'license:' prefix to the other packages in this file.
>
> Okay!
>
>> * gnu/packages/disk.scm (gparted): New variable.
>> (parted, fdisk, ddrescue, dosfstools, sdparm, idle3-tools): Avoid ambiguity by
>> adding a prefix to license symbols.
>
> It runs and finds my partitions! I didn't try to actually do anything...

I tested with a USB drive.  I could create a new partition table and a
new partition.

>> +    (arguments
>> +     `(#:tests? #f ; Tests require a network connection.
>
> Is it just a small subset of the tests that require the network? Maybe
> we could disable only those tests?

It seems all tests need to download DTDs from various places on the
internet.  The tests are related to the use of Scrollkeeper (which I
disabled).

As far as I can see there are no unit tests for the actual code.

>> +       #:configure-flags '("--disable-scrollkeeper")))
>
> I'm not sure what this means. Can you leave a comment explaining it?

Of course!  I'm not completely familiar with Scrollkeeper (although I
also wrote a package recipe for it), but what I understand is that it is
a program to manage various formats of documentation, and keep a central
database that links to various forms of documentation.

I added the following comment, but I think explaining the ins and outs of
Scrollkeeper goes too far.  Is the comment good enough?

+       ;; We don't use scrollkeeper elsewhere, so disable updating the
+       ;; scrollkeeper database with documentation from GParted.
+       #:configure-flags '("--disable-scrollkeeper")))

>> +    (inputs
>> +     `(("util-linux" ,util-linux)
>
> Since util-linux is just a mish-mash of miscellaneous utilities, I like
> it when packagers leave a hint of what it's being used for, either in a
> comment, or by naming it after the used library, as in the jack-1
> package.

I followed the example of jack-1 here, because for GParted, it's also
libuuid it takes from the util-linux package.

Kind regards,
Roel Janssen
Alex Kost May 28, 2016, 7:15 p.m. UTC | #3
Roel Janssen (2016-05-28 21:34 +0300) wrote:

> Hello Leo,
>
> Thank you for your quick response.
>
> Leo Famulari writes:
>
>> On Sat, May 28, 2016 at 05:52:16PM +0200, Roel Janssen wrote:
>>> Dear Guix,
>>> 
>>> I wonder if my ChangeLog-format commit message is OK this way.
>>
>> How about wording it as in bf50f7b5870? I don't think it's necessary to
>> list the affected packages if it applies to the entire module.
>
> Right.  I had looked for a similar change but could not find one.
> Thanks for pointing this one out.
>
> Would it perhaps be better if I split the license prefix changes and
> the addition of gparted in two separate patches?

Yes, these are 2 different things, so separating them would be the best,
I think.
Leo Famulari May 28, 2016, 7:28 p.m. UTC | #4
On Sat, May 28, 2016 at 08:34:19PM +0200, Roel Janssen wrote:
> Leo Famulari writes:
> > On Sat, May 28, 2016 at 05:52:16PM +0200, Roel Janssen wrote:
> I tested with a USB drive.  I could create a new partition table and a
> new partition.

Great!

> It seems all tests need to download DTDs from various places on the
> internet.  The tests are related to the use of Scrollkeeper (which I
> disabled).
> 
> As far as I can see there are no unit tests for the actual code.

Ah, too bad. I guess it relies on the tests for parted / libparted.

> >> +       #:configure-flags '("--disable-scrollkeeper")))
> >
> > I'm not sure what this means. Can you leave a comment explaining it?
> 
> Of course!  I'm not completely familiar with Scrollkeeper (although I
> also wrote a package recipe for it), but what I understand is that it is
> a program to manage various formats of documentation, and keep a central
> database that links to various forms of documentation.
> 
> I added the following comment, but I think explaining the ins and outs of
> Scrollkeeper goes too far.  Is the comment good enough?
> 
> +       ;; We don't use scrollkeeper elsewhere, so disable updating the
> +       ;; scrollkeeper database with documentation from GParted.
> +       #:configure-flags '("--disable-scrollkeeper")))

I think the comment is fine.

On a related topic, trying to open the help from the "Help" menu gives
me a dialog box this text:

"Unable to open GParted Manual help file

Failed to execute child process "yelp" (No such file or directory)"

I get the same result from the Debian package. I wonder if I would have
more success if I was running GNOME?

Since the man page doesn't include any information about using the
program, it would be good to make sure the help application at least
works on GNOME.
Roel Janssen May 28, 2016, 7:41 p.m. UTC | #5
Leo Famulari writes:

> On Sat, May 28, 2016 at 08:34:19PM +0200, Roel Janssen wrote:
>> Leo Famulari writes:
>> > On Sat, May 28, 2016 at 05:52:16PM +0200, Roel Janssen wrote:
>> I tested with a USB drive.  I could create a new partition table and a
>> new partition.
>
> Great!
>
>> It seems all tests need to download DTDs from various places on the
>> internet.  The tests are related to the use of Scrollkeeper (which I
>> disabled).
>> 
>> As far as I can see there are no unit tests for the actual code.
>
> Ah, too bad. I guess it relies on the tests for parted / libparted.

Yes, GParted is only a graphical user interface on top of libparted and
optionally external partitioning command-line tools.

>> >> +       #:configure-flags '("--disable-scrollkeeper")))
>> >
>> > I'm not sure what this means. Can you leave a comment explaining it?
>> 
>> Of course!  I'm not completely familiar with Scrollkeeper (although I
>> also wrote a package recipe for it), but what I understand is that it is
>> a program to manage various formats of documentation, and keep a central
>> database that links to various forms of documentation.
>> 
>> I added the following comment, but I think explaining the ins and outs of
>> Scrollkeeper goes too far.  Is the comment good enough?
>> 
>> +       ;; We don't use scrollkeeper elsewhere, so disable updating the
>> +       ;; scrollkeeper database with documentation from GParted.
>> +       #:configure-flags '("--disable-scrollkeeper")))
>
> I think the comment is fine.
>
> On a related topic, trying to open the help from the "Help" menu gives
> me a dialog box this text:
>
> "Unable to open GParted Manual help file
>
> Failed to execute child process "yelp" (No such file or directory)"
>
> I get the same result from the Debian package. I wonder if I would have
> more success if I was running GNOME?
>
> Since the man page doesn't include any information about using the
> program, it would be good to make sure the help application at least
> works on GNOME.

I am going to look further into this.  Thanks for exposing this
limitation (I hadn't tested this myself).

Kind regards,
Roel Janssen
diff mbox

Patch

diff --git a/gnu/packages/disk.scm b/gnu/packages/disk.scm
index c7aa0dc..5088f3c 100644
--- a/gnu/packages/disk.scm
+++ b/gnu/packages/disk.scm
@@ -4,6 +4,7 @@ 
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2016 Tobias Geerinckx-Rice <tobias.geerinckx.rice@gmail.com>
 ;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016 Roel Janssen <roel@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,17 +22,24 @@ 
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (gnu packages disk)
-  #:use-module (guix licenses)
+  #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
   #:use-module (guix build-system gnu)
+  #:use-module (gnu packages base)
+  #:use-module (gnu packages docbook)
   #:use-module (gnu packages gettext)
+  #:use-module (gnu packages glib)
+  #:use-module (gnu packages gtk)
+  #:use-module (gnu packages gnome)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages perl)
+  #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages readline)
   #:use-module (gnu packages guile)
-  #:use-module (gnu packages compression))
+  #:use-module (gnu packages compression)
+  #:use-module (gnu packages xml))
 
 (define-public parted
   (package
@@ -69,7 +77,7 @@ 
     (description
      "GNU Parted is a package for creating and manipulating disk partition
 tables.  It includes a library and command-line utility.")
-    (license gpl3+)))
+    (license license:gpl3+)))
 
 (define-public fdisk
   (package
@@ -95,7 +103,7 @@  tables.  It includes a library and command-line utility.")
      "GNU fdisk provides a GNU version of the common disk partitioning tool
 fdisk.  fdisk is used for the creation and manipulation of disk partition
 tables, and it understands a variety of different formats.")
-    (license gpl3+)))
+    (license license:gpl3+)))
 
 (define-public ddrescue
   (package
@@ -118,7 +126,7 @@  tables, and it understands a variety of different formats.")
 from one file to another, working to rescue data in case of read errors.  The
 program also includes a tool for manipulating its log files, which are used
 to recover data more efficiently by only reading the necessary blocks.")
-    (license gpl3+)))
+    (license license:gpl3+)))
 
 (define-public dosfstools
   (package
@@ -145,7 +153,7 @@  to recover data more efficiently by only reading the necessary blocks.")
     (description
      "The dosfstools package includes the mkfs.fat and fsck.fat utilities,
 which respectively make and check MS-DOS FAT filesystems.")
-    (license gpl3+)))
+    (license license:gpl3+)))
 
 (define-public sdparm
   (package
@@ -171,7 +179,7 @@  uses a SCSI command set.  Such devices include CD/DVD drives (irrespective of
 transport), SCSI and ATAPI tape drives, and SCSI enclosures.  This utility can
 also send commands associated with starting and stopping the media, loading
 and unloading removable media and some other housekeeping functions.")
-    (license bsd-3)))
+    (license license:bsd-3)))
 
 (define-public idle3-tools
   (package
@@ -203,4 +211,43 @@  present in many Western Digital hard drives.  This timer is part of the
 \"IntelliPark\" feature that stops the disk when not in use.  Unfortunately,
 the default timer setting is not well suited to Linux or other *nix systems,
 and can dramatically shorten the lifespan of the drive if left unchecked.")
-    (license gpl3+)))
+    (license license:gpl3+)))
+
+(define-public gparted
+  (package
+    (name "gparted")
+    (version "0.26.0")
+    (source
+     (origin
+       (method url-fetch)
+       (uri (string-append "mirror://sourceforge/gparted/gparted-"
+                           version ".tar.gz"))
+       (sha256
+        (base32 "1d3zw99yd1v0gqhcxff35wqz34xi6ps7q9j1bn11sghqihr3kwxw"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f ; Tests require a network connection.
+       #:configure-flags '("--disable-scrollkeeper")))
+    (inputs
+     `(("util-linux" ,util-linux)
+       ("parted" ,parted)
+       ("glib" ,glib)
+       ("gtkmm" ,gtkmm-2)
+       ("libxml2" ,libxml2)
+       ("libxslt" ,libxslt)
+       ("gnome-doc-utils" ,gnome-doc-utils)
+       ("docbook-xml" ,docbook-xml-4.2)
+       ("python" ,python-2)
+       ("python-libxml2" ,python2-libxml2)
+       ("which" ,which)))
+    (native-inputs
+     `(("intltool" ,intltool)
+       ("pkg-config" ,pkg-config)))
+    (home-page "https://sourceforge.net/projects/gparted/")
+    (synopsis "Partition editor to graphically manage disk partitions")
+    (description "GParted is a GNOME partition editor for creating,
+reorganizing, and deleting disk partitions.  It uses libparted from the parted
+project to detect and manipulate partition tables.  Optional file system tools
+permit managing file systems not included in libparted.")
+    ;; The home page says GPLv2, but the source code says GPLv2+.
+    (license license:gpl2+)))