[PATCH] Package Definition for QDirStat

OpenSubmitted by Thovthe.
Details
3 participants
  • Thovthe
  • Jack Hill
  • Jakub Kądziołka
Owner
unassigned
Severity
normal
T
T
Thovthe wrote on 3 May 02:28 +0200
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
27uwM7nMqpAz-RfKtxXzIOJWs0UnmDO3vqxoqkN1VeXR7GHQJTCgM5B0f8bCMuG2y5QsbCDj_AScqxM-4c1NO5YTf-ZAg3psDt-_FC27LJI=@protonmail.com
Here is a package definition for QDirStat, an excellent tool forchecking what files are taking up space on your system.  This ismy first guix module and it was made with help from Ryan Prior(github.com/sponsors/ryanprior). I have confirmed that it buildsand runs on aarch64 but the only system I have available is thisfairly anemic arm laptop so I haven't tested the otherarchitectures. Suggestions, testing, questions, comments andeducation welcomed.


From 9e56b8c2f5602c20ee5796de7af019ef640167cb Mon Sep 17 00:00:00 2001From: Thovthe <thovthe@protonmail.com>Date: Sat, 2 May 2020 22:48:30 +0000Subject: [PATCH] 2020-05-02 Thovthe thovthe@protonmail.com
* added new package definition for QDirStat at gnu/packages/qdirstat.scm--- gnu/packages/qdirstat.scm | 46 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 gnu/packages/qdirstat.scm
Toggle diff (52 lines)diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scmnew file mode 100644index 0000000000..91581a206e--- /dev/null+++ b/gnu/packages/qdirstat.scm@@ -0,0 +1,46 @@+(define-module (qdirstat)+ #: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))+++(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"))))))))++ (inputs+ `(("qtbase" ,qtbase)))+ (native-inputs+ `(("zlib" ,zlib)+ ("qttools" ,qttools)))+ (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." )+ (license license:gpl2+)))
--2.26.1
J
J
Jack Hill wrote on 5 May 06:01 +0200
(name . Thovthe)(address . Thovthe@protonmail.com)(address . 41040@debbugs.gnu.org)
alpine.DEB.2.20.2005042239310.5735@marsh.hcoop.net
Thovthe,
On Sun, 3 May 2020, Thovthe via Guix-patches via wrote:
Toggle quote (9 lines)> Here is a package definition for QDirStat, an excellent tool for> checking what files are taking up space on your system.  This is> my first guix module and it was made with help from Ryan Prior> (github.com/sponsors/ryanprior). I have confirmed that it builds> and runs on aarch64 but the only system I have available is this> fairly anemic arm laptop so I haven't tested the other> architectures. Suggestions, testing, questions, comments and> education welcomed.
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.
Toggle quote (7 lines)> From 9e56b8c2f5602c20ee5796de7af019ef640167cb Mon Sep 17 00:00:00 2001> From: Thovthe <thovthe@protonmail.com>> Date: Sat, 2 May 2020 22:48:30 +0000> Subject: [PATCH] 2020-05-02 Thovthe thovthe@protonmail.com>> * added new package definition for QDirStat at gnu/packages/qdirstat.scm
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.```
Toggle quote (11 lines)> ---> gnu/packages/qdirstat.scm | 46 +++++++++++++++++++++++++++++++++++++++> 1 file changed, 46 insertions(+)> create mode 100644 gnu/packages/qdirstat.scm>> diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scm> new file mode 100644> index 0000000000..91581a206e> --- /dev/null> +++ b/gnu/packages/qdirstat.scm
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
Toggle quote (3 lines)> @@ -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. You can test the patches locally on your end by applying them to a git checkout of the guix repository and running guix from that checkout. See the manual pages on Building from Git and Running Guix Before it is Installed.
https://guix.gnu.org/manual/en/html_node/Building-from-Git.htmlhttps://guix.gnu.org/manual/en/html_node/Running-Guix-Before-It-Is-Installed.html
Toggle quote (7 lines)> + #: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.
Toggle quote (25 lines)> +(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
Toggle quote (4 lines)> +> + (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.
Toggle quote (3 lines)> + (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 compiling.
Toggle quote (8 lines)> + ("qttools" ,qttools)))> + (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". Also there is an extra space between the closing quote and closing parentheses.
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."
Toggle quote (2 lines)> + (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 dependencies.
Can you send an updated patch?
Thanks again,Jack
J
J
Jack Hill wrote on 14 May 02:36 +0200
(name . Thovthe)(address . Thovthe@protonmail.com)(address . 41040@debbugs.gnu.org)
alpine.DEB.2.20.2005132028580.5735@marsh.hcoop.net
On Tue, 5 May 2020, Jack Hill wrote:
Toggle quote (16 lines)> Thovthe,>> On Sun, 3 May 2020, Thovthe via Guix-patches via wrote:>>> Here is a package definition for QDirStat, an excellent tool for>> checking what files are taking up space on your system.  This is>> my first guix module and it was made with help from Ryan Prior>> (github.com/sponsors/ryanprior). I have confirmed that it builds>> and runs on aarch64 but the only system I have available is this>> fairly anemic arm laptop so I haven't tested the other>> architectures. Suggestions, testing, questions, comments and>> education welcomed.>> 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.
Thovthe,
I'd like to follow up to see if you've had a chance to consider my comments. No pressure to work on it right now, but if you need help or have additional questions, just let us know. It will be great to get this package in Guix.
Best,Jack
T
T
Thovthe wrote on 4 Jun 09:40 +0200
(name . 41040@debbugs.gnu.org)(address . 41040@debbugs.gnu.org)
Nc6dd4WaVM3RhEkgX7S588RvUDKStBk52aTYK0lPdSpgZDRY7FVK-PxNfNVy8OUaKJcYe9eYszXFCIOS7ppMQvaVdyVxUy-ByiHoCjSTsf0=@protonmail.com
Toggle quote (3 lines)> 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.


Toggle quote (9 lines)> 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?


Toggle quote (3 lines)> 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 goesunder GNU_SYSTEM_MODULES.


Toggle quote (9 lines)> > @@ -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.
Done.
It was originally called that but the linter told me I ought to changeit. Did it do that because I worked on this outside of git?


Toggle quote (9 lines)> > - #: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 thebottom. If there is a convention feel free to change that around.


Toggle quote (28 lines)> > +(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.


Toggle quote (8 lines)> > - (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> compiling.
Done.


Toggle quote (10 lines)> > - (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 bebroken up at 78chars. Also makes M-q in emacs behave itself.
Toggle quote (2 lines)> Also there is an extra space between the closing quote and closing parentheses.
Blaming this on my tools as well possibly even the linter but I cant exactly recall what did it.


Toggle quote (6 lines)> 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 ratherthan inspection.


Toggle quote (10 lines)> > - (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
Done.


Toggle quote (5 lines)> 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 thisqdirstat.scm?


Toggle quote (9 lines)> > -> > - (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 aseparate package/output since this definition works for the essentialfunctionality.


Toggle quote (2 lines)> Can you send an updated patch?
Here and done!


```From f234ad07e06a5e97fdf1cad36e99e180ae6001f3 Mon Sep 17 00:00:00 2001From: Thovthe <thovthe@protonmail.com>Date: Thu, 4 Jun 2020 06:42:54 +0000Subject: [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(-)
Toggle diff (54 lines)diff --git a/gnu/local.mk b/gnu/local.mkindex 3c9a10b6bc..d1adcbeaa2 100644--- a/gnu/local.mk+++ b/gnu/local.mk@@ -440,6 +440,7 @@ GNU_SYSTEM_MODULES = \ %D%/packages/toys.scm \ %D%/packages/tryton.scm \ %D%/packages/qt.scm \+ %D%/packages/qdirstat.scm \ %D%/packages/radio.scm \ %D%/packages/ragel.scm \ %D%/packages/rails.scm \diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scmindex 91581a206e..a450b8a51a 100644--- a/gnu/packages/qdirstat.scm+++ b/gnu/packages/qdirstat.scm@@ -1,10 +1,10 @@-(define-module (qdirstat)- #: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:))

(define-public qdirstat@@ -33,14 +33,14 @@ (assoc-ref outputs "out"))))))))
(inputs- `(("qtbase" ,qtbase)))+ `(("qtbase" ,qtbase)+ ("zlib" ,zlib))) (native-inputs- `(("zlib" ,zlib)- ("qttools" ,qttools)))+ `(("qttools" ,qttools))) (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." )- (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
--2.26.2
```
J
J
Jack Hill wrote on 7 Jun 06:23 +0200
(name . Thovthe)(address . Thovthe@protonmail.com)(name . 41040@debbugs.gnu.org)(address . 41040@debbugs.gnu.org)
alpine.DEB.2.20.2006062341220.5735@marsh.hcoop.net
On Thu, 4 Jun 2020, Thovthe via Guix-patches via wrote:
Toggle quote (5 lines)>> 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.
Thanks! The patch looks improved to me. However, I'm still new to reviewing patches, so I don't know the answers to some of your questions. I also realized that I gave you bad advice is some places. Apologies, and thanks for bearing with me.
A more experienced person will be the one to commit the patch, and I'm sure that they will correct anything else I got wrong.
Toggle quote (11 lines)>> 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?
I'm not sure what you mean by smudge, but I think the answer is to amend. The idea is to have one commit that goes from having no QDirStat at all to working QDirStat package reflecting the improvements made during this review.
Toggle quote (6 lines)>> 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> under GNU_SYSTEM_MODULES.
I don't know why it is called GNU_SYSTEM_MODULES.
Toggle quote (14 lines)>>> @@ -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.>> Done.>> 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?
Yes, I think that's exactly it. Since the module name mirrors the relative file system path it is sensitive to those types of changes.
Toggle quote (12 lines)>>> - #: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.
Thanks. How we prefer to handle the guix licenses case is another thing that I don't know.
[…]
Toggle quote (18 lines)>>> - (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.
What I meant was that the lambda should return the value #t. Sometime we ensure this by adding a literal #t as the last form in the body of the lambda like:
(lambda (arg) (do-interesting-stuff …) ; this returns some value other than #t #t)
However, it turns out that invoke always returns #t on its own, so what you had originally was fine. Sorry for the confusion.
Toggle quote (10 lines)>>> - (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>> compiling.>> Done.
Great!
Toggle quote (13 lines)>>> - (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.
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.
Toggle quote (4 lines)>> Also there is an extra space between the closing quote and closing parentheses.>> Blaming this on my tools as well possibly even the linter but I cant exactly recall what did it.
:)
Toggle quote (9 lines)>> 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> than inspection.
Great, thank you.
Toggle quote (12 lines)>>> - (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>> Done.
Thanks.
Toggle quote (8 lines)>> 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.
Toggle quote (13 lines)>>> ->>> - (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> functionality.
Ok.
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!
Thanks again for your patience,Jack
T
T
Thovthe wrote on 13 Jun 23:51 +0200
(name . 41040@debbugs.gnu.org)(address . 41040@debbugs.gnu.org)
zAApckKRhqvy2njcWIMRnuUCNPFhXjm8fmOPU63f2CiFaXZY21i30yHZ1MUcnKXngDew7bcus_Z72EFO4L5Ty3z1rc-Bbj24AB6y-QXCx5o=@protonmail.com
Toggle quote (4 lines)> 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 ofaesthetics.
I've also moved some things around and added a line of white space for readabilityand separation of concerns.


Toggle quote (20 lines)> > > 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 figuringout how this package is built at a lower level since with this definitionprovides the essential functionality and all tests pass.


Toggle quote (10 lines)> 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 2001From: Thovthe <thovthe@protonmail.com>Date: Sat, 13 Jun 2020 21:42:38 +0000Subject: [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
Toggle diff (67 lines)diff --git a/gnu/local.mk b/gnu/local.mkindex aa5b67ab5f..2d06d1030c 100644--- a/gnu/local.mk+++ b/gnu/local.mk@@ -447,6 +447,7 @@ GNU_SYSTEM_MODULES = \ %D%/packages/python-xyz.scm \ %D%/packages/toys.scm \ %D%/packages/tryton.scm \+ %D%/packages/qdirstat.scm \ %D%/packages/qt.scm \ %D%/packages/radio.scm \ %D%/packages/ragel.scm \diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scmnew file mode 100644index 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:))+++(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+ (inputs+ `(("qtbase" ,qtbase) ; Could add ("perl" ,perl) as an input+ ("zlib" ,zlib))) ; for the cache-writer see debbug#41040+ (native-inputs+ `(("qttools" ,qttools)))+ (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"))))))))++ (home-page "https://github.com/shundhammer/qdirstat")+ (synopsis "Graphical disk space inspection utility")+ (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
--2.26.2
J
J
Jakub Kądziołka wrote on 28 Jun 14:56 +0200
(name . Thovthe)(address . Thovthe@protonmail.com)(name . 41040@debbugs.gnu.org)(address . 41040@debbugs.gnu.org)
20200628125624.3vr2borezix5ij4s@gravity
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 youinvestigate why this happens?
On Sat, Jun 13, 2020 at 09:51:29PM +0000, Thovthe wrote:
Toggle quote (24 lines)> > > > 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 qdirstatproper anyway.
It does make sense, however, to include perl as an input. This makes thescript work even though /usr/bin/perl doesn't exist on Guix System.
Toggle quote (13 lines)> >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 commentsafter the --- here and send the mail with git send-email (from thesend-email output of the git package), it would get picked upautomatically. Sending the patch as an attachment also works. However,pasting the patch into the message requires quite some fiddling with'git am'...
Toggle quote (14 lines)> 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 otherfiles.
Toggle quote (16 lines)> +(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 defaultvalue.
Regards,Jakub Kądziołka
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE5Xa/ss9usT31cTO54xWnWEYTFWQFAl74k3gACgkQ4xWnWEYTFWTLUA/+PkJz7GjI6VxjT9OLRy7bwGXIGq0CRJ4e1HSe1Idkfc2xAn+fzhbsRE9ATx5UeuHGQcWf0kuWJGS1wQ/aCp2+6I9QHsCGivjxEoqq5V7jrnnjGHCz3gJQ/e2r6wUQ0uO7RMau+uaSYK/0LlNt1eATyO02EbGFv7eBbQ4KkhP3mVocQ52TEv6+1aq9uQn1UBJGqZ9DJqhCdou6kaGVzxaeUQv1Fy30mYAUPRLkFpdHZM6ZdWfv58gLCKTiciXINDGLxtikhW4Ab3863AXpgJ6AYu3JijM5yFLjd3D2FzJFMUcsLSfnPxRf6WbOQSYNlTj+fOT+Kph5L9F7Zh2nwuuBsvsZIi+1bKrNDl5teWsRngckOeBZp1+r5rvSeneSIMZd4I+6xXuRIGkh1iv76CApnOVqKSprkab7VuGsdTNv2iS2iHecMdp5AD7ba3AcxgG1ayYToqa1hb10FiD3dQ7bNyxzCAXrNgLVt+UZukDaapUuTP8MikSdIBnKEAiMDVVdA5rpJGGTQnT+qIAW/Bx8g76cZ7Rk2MhIejCkNuci1VyfD5V0xfuENVokokpK1IrHSD3AG5ORnfXFXcVkLUiggdlI3v7frd2/pdoz0tmZnJi4sEZSIinmxTLz25vkXO0PQBWeRV7zaRclbURp3K9GtgBHsZiTptoLueOg7JE60dU==k9dq-----END PGP SIGNATURE-----

?