[bug#61207] gnu: Add opentaxsolver.

Hello, everyone,

With tax season coming up in the US, I wanted to use OpenTaxSolver to
prepare my tax return. However, it wasn't packaged on Guix!

OTS assumes it'll be run directly from an unpacked tarball, so a patch
is also included to fix some of the issues that causes (namely that an
unmodified build will try to save files to the store instead of the
user's home directory).

This is my first time trying to package something for Guix, so if there
are any glaring issues or mistakes here, please let me know!

Skylar Hill
From ec431fbcc49c8bb57ea6c11bb3fb4f5a93aa1a10 Mon Sep 17 00:00:00 2001
From: Skylar Hill <stellarskylark@posteo.net>
Date: Tue, 31 Jan 2023 22:30:25 -0600
Subject: [PATCH] gnu: Add opentaxsolver.

* gnu/packages/opentaxsolver.scm (opentaxsolver): New variable.
---
 gnu/packages/opentaxsolver.scm                | 94 +++++++++++++++++++
 .../opentaxsolver-file-browser-fix.patch      | 58 ++++++++++++
 2 files changed, 152 insertions(+)
 create mode 100644 gnu/packages/opentaxsolver.scm
 create mode 100644 gnu/packages/patches/opentaxsolver-file-browser-fix.patch

Hi Skylar,

Skylar Hill <stellarskylark@posteo.net> writes:

> Hello, everyone,
>
> With tax season coming up in the US, I wanted to use OpenTaxSolver to
> prepare my tax return. However, it wasn't packaged on Guix!
>
> OTS assumes it'll be run directly from an unpacked tarball, so a patch
> is also included to fix some of the issues that causes (namely that an
> unmodified build will try to save files to the store instead of the
> user's home directory).
>
> This is my first time trying to package something for Guix, so if there
> are any glaring issues or mistakes here, please let me know!

Thanks for your work!  Adding a new package is a great first
contribution.  Here are some suggestions:

> Skylar Hill
>
> From ec431fbcc49c8bb57ea6c11bb3fb4f5a93aa1a10 Mon Sep 17 00:00:00 2001
> From: Skylar Hill <stellarskylark@posteo.net>
> Date: Tue, 31 Jan 2023 22:30:25 -0600
> Subject: [PATCH] gnu: Add opentaxsolver.
>
> * gnu/packages/opentaxsolver.scm (opentaxsolver): New variable.
> ---
>  gnu/packages/opentaxsolver.scm                | 94 +++++++++++++++++++
>  .../opentaxsolver-file-browser-fix.patch      | 58 ++++++++++++
>  2 files changed, 152 insertions(+)
>  create mode 100644 gnu/packages/opentaxsolver.scm

We don't usually create new files for specific packages.  I'd suggest
putting this definition into finance.scm.

>  create mode 100644 gnu/packages/patches/opentaxsolver-file-browser-fix.patch
> [...]
> +
> +;; The version number seems to consist of two separate parts, a typical
> +;; version number (ots-version) and a tax year.  At time of writing, the
> +;; version is 2022_20.00.  Both parts are used separately in the tarball uri,
> +;; it is convenient to define both as separate variables.
> +
> +(define tax-year "2022")
> +(define ots-version "20.00")
> +
> +(define-public opentaxsolver

What I would do here instead would be to let-bind tax-year and
ots-version in the body of the define-public clause, so that it's only
defined there.

> +  (package
> +    (name "opentaxsolver")
> +    (version (string-append tax-year "_" ots-version))
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "mirror://sourceforge/opentaxsolver/OTS_"
> +                                  tax-year
> +                                  "/v"
> +                                  ots-version
> +                                  "_linux/OpenTaxSolver"
> +                                  version
> +                                  "_linux64.tgz"))
> +              (sha256
> +               (base32
> +                "06k0a72bmwdmr71dvrp8b4vl8vilnggsh92hrp7wjdgcjj9m074w"))
> +              (patches (search-patches "opentaxsolver-file-browser-fix.patch"))))
> +    (build-system glib-or-gtk-build-system)
> +
> +    (arguments
> +     '(#:phases (modify-phases %standard-phases
> +                  (delete 'configure)
> +                  (replace 'build
> +                    (lambda _
> +                      ;; The provided Build_taxsolve_packages.sh would do this, but
> +                      ;; then we can't configure CC=gcc in the make calls, which
> +                      ;; errors.  Also, the provided `make clean` doesn't delete the
> +                      ;; GUI.
> +                      (delete-file "Run_taxsolve_GUI")
> +                      (delete-file-recursively "bin")
> +                      (mkdir "bin")
> +                      (chdir "src/Gui_gtk")
> +                      (invoke "make" "CC=gcc")

Here, you shouldn't directly talk about gcc, since the compiler name
might be different when cross-compiling.  We have a procedure for that,
cc-for-target, an example use would be (invoke "make" (string-append
"CC=" ,(cc-for-target))), but you would need to replace the quote
'(#:phases ...) to a quasiquote `(#:phases ...). You should have a look
at quote and quasiquote in the Guile info manual, node "Expression
Syntax".

By the way, we now use g-expressions instead of s-expressions for
phases, but that might be too big of a commitment for a first
contribution.  Someone else can adjust that part after these changes.
You can still have a look in the Guix manual for the G-Expressions node
if you're so inclined.

> +                      (chdir "..")
> +                      (invoke "make" "CC=gcc")))

Ditto.

> +                  (delete 'check)
> +                  (replace 'install
> +                    (lambda* (#:key outputs #:allow-other-keys)
> +                      (let* ((out (assoc-ref outputs "out"))
> +                             (bin (string-append out "/bin")))
> +                        ;; OTS was designed to be run straight out of the unpacked
> +                        ;; tarball. Thus, the installation procedure is extremely
> +                        ;; cursed. Also note we don't use the provided
> +                        ;; Run_taxsolve_GUI because it's pointless in this context.
> +                        (copy-recursively "../bin" bin)
> +                        (copy-recursively "../tax_form_files"
> +                                          (string-append out "/tax_form_files"))
> +                        (copy-recursively "formdata"
> +                                          (string-append out "/src/formdata"))))))))
> +    (inputs (list (specification->package "gtk+@2")))

You shouldn't use specification->package in package definitions, as
that's pretty costly (it needs to scan the whole package list for
it). You should instead make the package variable available itself by
using (#:use-module (gnu packages gtk)), and use the variable gtk+-2.

> +    (native-inputs (list gcc pkg-config))

There's no need to add gcc to the list of native-inputs, it is included
by most build systems, glib-or-gtk included.

> +    (synopsis "Yearly tax preparation tool")
> +    (description
> +     "OpenTaxSolver is a free, safe, and secure program for calculating tax form entries for federal and state personal income taxes.  It automatically fills out and prints your forms for mailing.
> +
> +Invoke with @code{ots_gui2} rather than the usual @code{Run_taxsolve_GUI}.")
> +    (home-page "https://opentaxsolver.sourceforge.net/")
> +    (license license:gpl2+)))
> diff --git a/gnu/packages/patches/opentaxsolver-file-browser-fix.patch b/gnu/packages/patches/opentaxsolver-file-browser-fix.patch
> new file mode 100644
> index 0000000000..0e6be74f85
> --- /dev/null
> +++ b/gnu/packages/patches/opentaxsolver-file-browser-fix.patch
> @@ -0,0 +1,58 @@
> +From 96fda11a53a89c6647031f2c05ef12f1a9df6a08 Mon Sep 17 00:00:00 2001
> +From: Skylar Hill <stellarskylark@posteo.net>
> +Date: Tue, 31 Jan 2023 21:02:18 -0600
> +Subject: [PATCH] Change default directory in file browser
> +
> +---
> + src/Gui_gtk/ots_gui2.c | 7 +++++--
> + 1 file changed, 5 insertions(+), 2 deletions(-)
> +
> +diff --git a/src/Gui_gtk/ots_gui2.c b/src/Gui_gtk/ots_gui2.c
> +index ff3366b..1247933 100644
> +--- a/src/Gui_gtk/ots_gui2.c
> ++++ b/src/Gui_gtk/ots_gui2.c
> +@@ -82,6 +82,7 @@ char ots_release_package[]="20.00";
> + #include <string.h>
> + #include <stdlib.h>
> + #include <ctype.h>
> ++#include <unistd.h>
> + #include <sys/stat.h>
> + // #include "backcompat.c"
> + #include "gtk_utils.c"		/* Include the graphics library. */
> +@@ -109,6 +110,7 @@ char toolpath[MaxFname]="", *start_cmd;
> + int pending_compute=0, supported_pdf_form=1;
> + int filingstatus_mfj=1;
> + int round_to_whole_nums=0;
> ++char *working_dir[MaxFname+512];
> + 
> + void pick_file( GtkWidget *wdg, void *data );	/* Prototype */
> + void consume_leading_trailing_whitespace( char *line );
> +@@ -2364,7 +2366,7 @@ void save_taxfile( GtkWidget *wdg, void *data )
> +  if (cpt != 0)
> +   strcpy( cpt, "_xxxx.txt" );
> +  // printf("OTS_save_taxfile: dir='%s', wc='%s', fname='%s'\n", directory_dat, wildcards_fb, filename_fb );
> +- Browse_Files( "File to Save As:", 2048, directory_dat, wildcards_fb, filename_fb, Save_Tax_File );
> ++ Browse_Files( "File to Save As:", 2048, working_dir, wildcards_fb, filename_fb, Save_Tax_File );
> + }
> + 
> + 
> +@@ -3878,7 +3880,7 @@ void pick_file( GtkWidget *wdg, void *data )
> +   strcpy( wildcards_fb, ".txt" );
> +   strcpy( filename_fb, "" );
> +   // printf("OTS_pick_file: dir='%s', wc='%s', fname='%s'\n", directory_dat, wildcards_fb, filename_fb );
> +-  Browse_Files( "Select File", 2048, directory_dat, wildcards_fb, filename_fb, receive_filename );
> ++  Browse_Files( "Select File", 2048, working_dir, wildcards_fb, filename_fb, receive_filename );
> + }
> + 
> + 
> +@@ -4019,6 +4021,7 @@ int main(int argc, char *argv[] )
> +  invocation_path[k] = '\0';
> +  // printf("Invocation path = '%s'\n", invocation_path);
> +  set_ots_path();
> ++ getcwd(working_dir, MaxFname+512);
> + 
> +  /* Decode any command-line arguments. */
> +  argn = 1;  k=1;
> +-- 
> +2.38.1
> +
> -- 
> 2.38.1
>

Best,

Hi Skylar,

Skylar Hill <stellarskylark@posteo.net> writes:

> Thanks, Josselin! Here's an updated patch including the requested
> changes. I also took a crack at switching over to G-expressions -- let
> me know if I need to fix anything on that front to match best practices
> or fully take advantage of the construct.
>
> Skylar Hill
>
> [...]

Thanks for the quick changes, this looks really good, although you
forgot to reply to the bug email address! I've added it again and will
resend the patch, I've only removed one extra whitespace and rebased on
top of master (there is a new copyright line where you added yours). I
am no committer though so someone else will have to look at this, and
there might be an issue with the license, although I'm no expert: I see
some files with a GPLv2 header, another with a LGPLv2 header, some with
none.

Also, it's good practice to include the fact that it's the second
version of a patch by adding --reroll-count=2 (shortened -v 2) to git
format/send-patch, and also adding the base commit you're using using
--base=auto.

Best,