Hi again!

Your patch applies perfectly, the error was on my side.

Also, a gdcm package was added to the bioinformatics module 4 days ago=) It
packages an older version (2.8.9) but is much more concise than my attempt.
Would it be worthwhile to continue work on this package as perhaps gdcm-3.0
and move it to bioinformatics? I put it in image-processing since dcmtk was
already there.

Best regards

Den ons 11 jan. 2023 kl 08:08 skrev Tor-björn Claesson <tclaesson@gmail.com
>:

>
> Hi!
>
> Tobias Geerinckx-Rice <me@tobias.gr> writes:
> >
> >> 3. It does not perform tests.
> >
> > OK, I'll take a look.
> >
> > If tests are disabled, the reason should always be noted in a comment.
> > Even if it's just ‘; no test suite’.
> >
>
> #:tests? #t makes the build fail with "make: *** No rule to make target
>  'test'.  Stop."
>
> GDCM has nightly regression tests
> (https://open.cdash.org/index.php?project=GDCM), should we try to run
> those when building? I have tried to find out how to do this but for now
> with no success. Maybe it is obvious to more experienced people?
>
> >> +(define-public gdcm
> >
> > It used to be common to unconditionally add packages to the end of
> > files, but this needlessly increased the risk of merge conflicts.
> >
> > Instead, just add them wherever they first fit alphabetically; here, I
> > put it above ‘mia’.
> >
>
> Ok, will do from now on!
>
> >> +    (version "3.0.20")
> >
> > ‘guix lint’ says this can be updated to 3.1.0 but I didn't try, as I'd
> > rather it be tested by an actual user — i.e., you.
> >
>
> I got that too, but the latest release in git is 3.0.20
>
> >> + "1w78cmm9q7aavs7svdkl4dgilcqk4yazci9m6x7icrssb7cj991i"))))
> >> +    (build-system cmake-build-system)
> >> +    (outputs '("out" "doc"))
> >
> > /share/doc wasn't actually installed into "doc", but to "out", so I
> > set the GDCM_INSTALL_DOC_DIR configure flag.
> >
> >> +    (arguments
> >> +     (list #:tests? #f
> >> +           #:phases #~(modify-phases %standard-phases
> >> +                        (add-before 'configure 'set-LDFLAGS
> >> +                          (lambda* (#:key inputs outputs
> >> #:allow-other-keys)
> >> +                            (setenv "LDFLAGS"
> >> +                                    (string-append "-Wl,-rpath="
> >> +                                                   #$output
> >> "/lib"))))
> >> +                        (add-before 'build 'patch-gdcm-charls.h
> >> +                          (lambda _
> >> +                            (substitute*
> >> "../source/Utilities/gdcm_charls.h"
> >> +                              (("# include <CharLS/charls.h>")
>
> Ah, good catch!
>
> >
> > Purely as a matter of taste I dropped the ‘# include ’ from both
> > strings and escaped the ‘.’ in the regexp.
> >
> >> +                               "# include <charls/charls.h>"))
> >> #t)))
> >
> > ‘#t’ endings are also obsolete.  Just drop them entirely.  Phases can
> > now safely return anything, including nothing or undefined.
> >
> > I added the following phase to work around log spam, since I didn't
> > find its source (nor did I look very hard) [edit: it was indeed
> > graphviz, thanks].  By default, $HOME is not writable in the build
> > environment.
> >
> >  (add-before 'build 'set-HOME
> >    ;; The build spams ‘Fontconfig error: No writable cache
> >    ;; directories’ in a seemingly endless loop otherwise.
> >    (lambda _
> >      (setenv "HOME" "/tmp")))
> >
> >> +           #:configure-flags #~(list "-DCMAKE_SKIP_RPATH:BOOL=YES"
>
> Is this needed, btw? It came from gdcm:s packaging
> instructions. Removing it causes no verify-runpath issues.
>
> >
> > I, opinionated, added newlines after #:phases and #:configure-flags.
> >
> > Some people like the ‘extreme indentation’ you get by throwing away
> > half of your screen width.  I find it leads to cramped code and noisy
> > patches once the phases need to get actual work done or an even longer
> > CMAKE_ flag comes along.
> >
> > I also added some newlines and tried to group related flags.
> >
>
> Thanks, I didn't know that would make the line fit better on
> screen. Much neater=)
>
> >> + "-DCMAKE_C_FLAGS=-fvisibility=hidden"
> >> + "-DCMAKE_CXX_FLAGS=-fvisibility=hidden"
> >
> > Should these be explained in a very brief comment?
> >
>
> They are from https://github.com/malaterre/GDCM/blob/master/PACKAGER,
> the explanation is:
> "This make sure that on UNIX, the API is actually identical at what is
> found on Windows."
>
> >
> > Thank you for building with system libraries!  Also remove the bundled
> > copies when possible.  I did so in a (rather strict) source snippet.
> >
>
> Ok, neat=)
>
> >> + "-DGDCM_PDF_DOCUMENTATION:BOOL=OFF"
> >
> > I cannot get the man pages to build, either.  They need something
> > called ‘xsl-ns’.  I've disabled GDCM_BUILD_DOCBOOK_MANPAGES for now.
> >
> >> + "-DGCM_BUILD_TESTING:BOOL=OFF"
> >
> > Why is this set?  It's reported by CMake as having no effect, and a
> > diff of the output confirms that.
> >
>
> From the old wiki:
>
> "This boolean is responsible for deciding whether or not to build/run the
> nightly regression test of gdcm. Warning when turning this option on,
> the size of the gdcm libraries will be bigger since some extra code are
> compiled in for the testing framework (see gdcm::Testing, and the md5
> lib)."
>
> This seems to be incorrect then, maybe we can skip it.
>
> >> +    (license license:bsd-3)))
> >
> > I still need to check this.
> >
>
> https://github.com/malaterre/GDCM/blob/master/Copyright.txt
>
> I'm not able to apply your new patch, but that is probably a fault on my
> part.
>
> Thanks a lot for sharing your time and knowledge, and for making this
> patch neater! I find this a lot of fun, but have no experience
> with scheme or packaging, so your explanations are very valuable to me.
>
> Cheers
> Tor-björn
>