Subject: Re: [bug#41040] [PATCH] Package Definition for QDirStat

From: Jakub Kądziołka <[email-masked]>
Date: Sun, 28 Jun 2020 14:56:24 +0200

Thanks for your patch!

The repository includes man pages, but the built package doesn't.
Interestingly, the man1 directory is there, it's just empty. Could you
investigate why this happens?

On Sat, Jun 13, 2020 at 09:51:29PM +0000, Thovthe wrote:
> > > > I think that perl should be added as input, so that the #! line of
> > > > qdirstat-cache-writer can be patched to refer to a perl in the store.
> > >
> > > I'm leaving this for once I've moved qdirstat-cache-writer into a
> > > separate package/output since this definition works for the essential
> > > functionality.
> >
> > Ok.
> > -------------------------
> > > > For bonus points, it might be nice to move qdirstat-cache-writer to its
> > > > own output since it is made to be run independently of QDirStat and that
> > > > way it could be installed without pulling in all the C++ and Graphical
> > > > dependencies.
> > >
> > > How do you think this should look? Would I make another output in this
> > > qdirstat.scm?
> >
> > Yes, adding another output is what I was thinking. The bind package is an
> > example of this.
> 
> I've added some comments about this but right now I'm not interested in figuring
> out how this package is built at a lower level since with this definition
> provides the essential functionality and all tests pass.

I don't think it makes much sense to separate it into another output,
given that the data produced by the script would be consumed by qdirstat
proper anyway.

It does make sense, however, to include perl as an input. This makes the
script work even though /usr/bin/perl doesn't exist on Guix System.

> >From 159a4f8b44537c8b76db52b65cd9571962cb58e4 Mon Sep 17 00:00:00 2001
> From: Thovthe <[email-masked]>
> Date: Sat, 13 Jun 2020 21:42:38 +0000
> Subject: [PATCH] gnu: Add qdirstat.
> 
> * gnu/packages/qdirstat.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
> ---
>  gnu/local.mk              |  1 +
>  gnu/packages/qdirstat.scm | 49 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 50 insertions(+)
>  create mode 100644 gnu/packages/qdirstat.scm

Applying this patch was a bit problematic. If you put your comments
after the --- here and send the mail with git send-email (from the
send-email output of the git package), it would get picked up
automatically. Sending the patch as an attachment also works. However,
pasting the patch into the message requires quite some fiddling with
'git am'...

> diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scm
> new file mode 100644
> index 0000000000..8188357855
> --- /dev/null
> +++ b/gnu/packages/qdirstat.scm
> @@ -0,0 +1,49 @@
> +(define-module (gnu packages qdirstat)
> +  #:use-module (gnu packages compression)
> +  #:use-module (gnu packages qt)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix git-download)
> +  #:use-module (guix packages)
> +  #:use-module ((guix licenses) #:prefix license:))

Make sure to include the copyright header that's present in all other
files.

> +(define-public qdirstat
> +  (package
> +    (name "qdirstat")
> +    (version "1.6.1")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/shundhammer/qdirstat.git")
> +             (commit version)))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32 "0q77a347qv1aka6sni6l03zh5jzyy9s74aygg554r73g01kxczpb"))))
> +
> +    (outputs `("out"))			; "cache-writer" could be made an output

There's no point in specifying OUTPUTS if we're going to use the default
value.

Regards,
Jakub Kądziołka