Subject: Re: [bug#59355] [PATCH] gnu: Add dataparksearch.

From: Tobias Geerinckx-Rice <[email-masked]>
Date: Sat, 18 Feb 2023 01:13:24 +0100

Hi Adam,

Adam Faiz via Guix-patches via 写道：
> * gnu/packages/search.scm (dataparksearch): New variable.

Thanks!  I've applied the patch locally but stopped short of 
pushing.  I have several questions and [notes]:

> +(define-public dataparksearch
> +  (package
> +    (name "dataparksearch")
> +    (version "4.54-2016-12-03")

I don't think this tag is anything special compared to other 
commits.  Is it?

There are many more recent commits, up to 2022.  If the argument 
is ‘4.53 is ancient’ (it is!), should we just package the latest 
commit?

Would the latest release (4.53) instead still be useful today?

> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url 
> "https://github.com/Maxime2/dataparksearch")
> +                    (commit version)))
> +              (sha256
> +               (base32
> "1g5kxw2d8jbc1h9yyy0xpnd3gkscj5a32k6hk3brvdwcbsnjbgyg"))
> +              (modules '((guix build utils)))
> +              (snippet
> +               #~(begin
> +                   (for-each delete-file-recursively 
> '("config.sub"
> + 
> "config.guess"
> + 
> "configure"

[All checked into Git.  Nice.  This snippet is ‘ugly’ because it 
addresses an ugly problem.  I think I'll keep it but add an 
apologetic comment.]

> + 
> "Makefile.in"
> + 
> "missing"
> + 
> "depcomp"
> + 
> "ltmain.sh"
> + 
> "compile"
> +                                                       ))))

[We don't dangle brackets like this but keep them on the previous 
line.]

> +              (file-name (git-file-name name version))))
> +    (build-system gnu-build-system)
> +    (native-inputs
> +     (list pkg-config automake autoconf libtool openjade))
> +    (inputs
> +     (list mbedtls-apache zlib postgresql aspell c-ares 
> libextractor))

[For consistency with other packages, I've moved the …inputs below 
the arguments field.]

> +     (list #:configure-flags
> +           #~(list "--disable-syslog"

Is this not useful?  If not, could you provide a short comment 
explaining why?

> +                   "--with-gnu-ld"

And here?

> +               (add-before 'bootstrap 'fix-configure
> +                 (lambda _
> +                   (substitute* "configure.ac"
> +                     (("MY_DIRS=\"/usr/local/include")
> +                      (string-append "MY_DIRS=\""
> #$(this-package-input "aspell") "/include"))
> +                     (("MY_DIRS=\"/usr/lib")
> +                      (string-append "MY_DIRS=\""
> #$(this-package-input "aspell") "/lib"))))))))

Why not use --with-aspell=… as you did for Postgres?

> +    (synopsis "Libre, feature rich search engine")

[We don't need to include ‘libre’ (or ‘Free’, ‘open-source’, …) in 
descriptions: there's no other software in Guix.  \o/ ]

> +    (description
> +     "Dataparksearch is a full featured web search engine.
> +It has support for http, https, ftp (passive mode), nntp and 
> news URL
> schemes, and
> +other URL schemes with external parsers.
> +It can tweak URLs with session IDs and other weird formats,
> +including some JavaScript link decoding.
> +Options to query with all words, all words near to each others, 
> any
> words, or Boolean queries.
> +A subset of VQL (Verity Query Language) is supported.")
> +    (home-page "https://www.dataparksearch.org/ ")

[Extra trailing ‘ ’ removed.]

> +    (license license:gpl3+)))

Why GPL3+?

[I was unable to get this package to build reproducibly, although 
I tried only disabling #:parallel-build?.]

Kind regards,

T G-R