[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#41040] [PATCH] Package Definition for QDirStat
[bug#41040] [PATCH] Package Definition for QDirStat
Thu, 04 Jun 2020 07:40:35 +0000
> Thanks for contributing this patch to Guix! I have some suggestions for
> how it can be cleaned up so that it can be included in the distribution.
Sounds good, I'll try to address those suggestions.
> Guix uses a very structured changelog format for commit messages. I find
> that the best way to figure it out is to look at the existing git log. In
> this case the commit message should be something like:
> gnu: Add qdirstat.
> * gnu/packages/qdirstat.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
I hope the new one works fine. Do you want me to amend or smudge?
> As you may have guessed from my proposed commit message, when adding a new
> file, you should also add it to the appropriate section in gnu/local.mk
I have done, but I must admit it is somewhat confusing to me that it goes
> > @@ -0,0 +1,46 @@
> > +(define-module (qdirstat)
> The module name should be (gnu packages qdirstat) to match the file path
> of the module.
> If you'll permit me to speculate a little bit: I suspect that you
> developed this package outside of the main Guix repository.
It was originally called that but the linter told me I ought to change
it. Did it do that because I worked on this outside of git?
> > - #:use-module (gnu packages qt)
> > - #:use-module (gnu packages compression)
> > - #:use-module (guix build-system gnu)
> > - #:use-module (guix git-download)
> > - #:use-module ((guix licenses) #:prefix license:)
> > - #:use-module (guix packages))
> Since this is a new file, let's go ahead and sort these alphabetically.
I have now but I'm not sure how to treat parens so the `licenses` line is at the
bottom. If there is a convention feel free to change that around.
> > +(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"))))
> > - (build-system gnu-build-system)
> > - (arguments
> > - `(#:phases
> > - (modify-phases %standard-phases
> > - (replace 'configure
> > - (lambda* (#:key outputs #:allow-other-keys)
> > - (invoke "qmake"
> > - (string-append "PREFIX="
> > - (assoc-ref outputs "out"))
> > - (string-append "INSTALL_PREFIX="
> > - (assoc-ref outputs "out"))))))))
> It is our custom to have all phases return #t
I don't really know what this means, looked through the documentation but all I
can workout is that something needs to pass what I assume is 'true' to
somewhere else. I took this verbatim from another definition in guix.
> > - (native-inputs
> > - `(("zlib" ,zlib)
> I think zlib should be an input rather than a native-input since the code
> seems to link to it. Remember that the reason for native-inputs verses
> inputs is that native-inputs are needed on the host side when cross
> > - (home-page "https://github.com/shundhammer/qdirstat")
> > - (synopsis "Graphical disk space inspection utility")
> > - (description
> > - "QDirStat is a graphical application to show where your disk space
> > has
> > +gone and to help you to clean it up. Shaded boxes represent files and files
> > +are grouped by directory structure." )
> Some formatting nitpicks: you can go ahead and start the description on
> the same line as "(description".
Done, though I think it looks better to have it all columnised if it needs to be
broken up at 78chars. Also makes M-q in emacs behave itself.
> Also there is an extra space between the closing quote and closing
Blaming this on my tools as well possibly even the linter but I cant exactly
recall what did it.
> The best terms I found when searching for similar packages that we already
> have in Guix where, "disk usage", so I think it would be good idea to work
> usage into the synopsis or description somehow. Perhaps change the
> synopsis to say, "disk usage inspection" or the description to say, "show
> your disk usage and help you clean it up."
Done and added analys- root word in case that comes to someone's mind rather
> > - (license license:gpl2+)))
> qdirstat-cache-writer has a different license in its header. It's an …
> interesting licence :) I think it's free software, so eligable for
> inclusion in Guix, but probably worth recording here, so that the licence
> field becomes:
> (license (list license:gpl2+
> license:non-copyleft)) ; scripts/qdirstat-cache-writer
> 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
How do you think this should look? Would I make another output in this
> > -
> > - (inputs
> > - `(("qtbase" ,qtbase)))
> 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
> Can you send an updated patch?
Here and done!
>From f234ad07e06a5e97fdf1cad36e99e180ae6001f3 Mon Sep 17 00:00:00 2001
From: Thovthe <email@example.com>
Date: Thu, 4 Jun 2020 06:42:54 +0000
Subject: [PATCH] gnu: Add qdirstat.
* gnu/packages/qdirstat.scm: Modified according to feedback
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
gnu/local.mk | 1 +
gnu/packages/qdirstat.scm | 24 ++++++++++++------------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/gnu/local.mk b/gnu/local.mk
index 3c9a10b6bc..d1adcbeaa2 100644
@@ -440,6 +440,7 @@ GNU_SYSTEM_MODULES = \
+ %D%/packages/qdirstat.scm \
diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scm
index 91581a206e..a450b8a51a 100644
@@ -1,10 +1,10 @@
- #:use-module (gnu packages qt)
+(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 licenses) #:prefix license:)
- #:use-module (guix packages))
+ #:use-module (guix packages)
+ #:use-module ((guix licenses) #:prefix license:))
@@ -33,14 +33,14 @@
(assoc-ref outputs "out"))))))))
- `(("qtbase" ,qtbase)))
+ `(("qtbase" ,qtbase)
+ ("zlib" ,zlib)))
- `(("zlib" ,zlib)
- ("qttools" ,qttools)))
+ `(("qttools" ,qttools)))
(synopsis "Graphical disk space inspection utility")
- "QDirStat is a graphical application to show where your disk space has
-gone and to help you to clean it up. Shaded boxes represent files and files
-are grouped by directory structure." )
- (license license:gpl2+)))
+ (description "QDirStat is a graphical application for analysing disk
usage. It shows
+where your disk space has gone and helps you clean it up. Shaded boxes
+represent files and files are grouped by directory structure.")
+ (license (list license:gpl2+
+ license:non-copyleft)))) ; scripts/qdirstat-cache-writer
- [bug#41040] [PATCH] Package Definition for QDirStat,