[bug#41040] Package Definition for QDirStat

> This is another thing that I got wrong. It looks like having the newline
> is a common thing to do in Guix, probably for the reasons you describe.
> Feel free to put this back as it was. Sorry for creating extra work.

Not a problem.  I'm happy that it's ok for it to closer match my sense of
aesthetics.

I've also moved some things around and added a line of white space for readability
and separation of concerns.



> > > 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.



> To summarize:
>
> Can you squash your squash all your changes together to send one patch?
>
> A more experienced Guix reviewer will have to answer the questions about
> the ordering of the module imports and the right thing to do with
> qdirstat-cache-writer.
>
> Once that's all done, they should be able to commit QDirStat to Guix!

Alright, it should look like it's popped into being from the very aether.





From 159a4f8b44537c8b76db52b65cd9571962cb58e4 Mon Sep 17 00:00:00 2001
From: Thovthe <thovthe@protonmail.com>
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

--
2.26.2

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 <thovthe@protonmail.com>
> 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