unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: Andreas Enge <andreas@enge.fr>
To: David Elsing <david.elsing@posteo.net>
Cc: 64446@debbugs.gnu.org, efraim@flashner.co.il, bavier@posteo.net
Subject: [bug#64446] [PATCH] gnu: Add bliss.
Date: Wed, 26 Jul 2023 18:10:27 +0200	[thread overview]
Message-ID: <ZMFFc6mBoKVRW8y8@jurong> (raw)
In-Reply-To: <5840cd3331640479ee55e604b4e5080be85819ba.1688419275.git.david.elsing@posteo.net>

Hello,

I have spent quite a long time on this package, trying to simplify your
recipe.

Am Mon, Jul 03, 2023 at 09:21:32PM +0000 schrieb David Elsing:
> +          (add-after 'unpack 'fix-string-macro
> +            (lambda _
> +              (substitute* "bliss.cc"
> +                (("\"__DATE__\"") "\" __DATE__ \""))))

This so far is only a warning with newer gcc versions, so we do not really
need it.

> +          ;; Move headers under the bliss/ prefix
> +          (add-after 'unpack 'move-headers
> +            (lambda _
> +              (substitute* (find-files "." "\\.(h|hh|cc)$")
> +                (("#include \"(.*)\"" all path)
> +                 (string-append "#include <bliss/" path ">")))
> +              (mkdir-p "bliss")
> +              (for-each
> +               (lambda (file)
> +                 (rename-file file
> +                              (string-append "bliss/" (basename file))))
> +               (find-files "." "\\.(h|hh)$"))))

All surprising phases need more comments for their rationale.
I added this:
          ;; Move headers under the bliss/ prefix. This is a Guix choice,
          ;; since the header names are sufficiently generic to cause
          ;; confusions with other packages ("heap.hh").

> +          (add-after 'move-headers 'disable-gmp
> +            (lambda _
> +              (substitute* "bliss/bignum.hh"
> +                (("defined\\(BLISS_USE_GMP\\)") "0"))))

This looks like it is not needed if using the Makefile.

> +          (replace 'build

Here I am not convinced. You end up rewriting the Makefile in Guile.
The Makefile works, but it tries to create a binary "bliss", which
collides with the new file for the headers. This could be solved by
moving the content of the 'move-headers phase between the installation
of the bliss binary (after which it can be deleted) and the installation
of the headers.

Moreover, the Makefile does not create a dynamic, but only a static
library, and your build phase adds a dynamic library. Is this our role
as packagers?

According to the time stamps of the files inside the .zip, the software
dates from 2015 and is apparently unmaintained (otherwise I would have
suggested to get in touch with the developers to improve the Makefile).

So I wonder whether this software meets the quality standards for inclusion
into Guix.

Hm, I just found a new version here:
   https://users.aalto.fi/~tjunttil/bliss/index.html :
"Compiling
In Linux and macOS, one can use GNU Make to compile the bliss executable, as well as the static and shared libraries, with (...)"!

And the author is here:
   https://users.aalto.fi/~tjunttil/

Would you like to give it another try, David? And maybe discuss with the
author whether they would be willing to implement the bliss/ subdirectory
for the headers? (Given that there are now separate src/ and build/
subdirectories that would be quite easy.) And add an "install" target?

Andreas





  reply	other threads:[~2023-07-26 16:19 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-03 21:21 [bug#64446] [PATCH] gnu: Add bliss David Elsing
2023-07-26 16:10 ` Andreas Enge [this message]
2023-09-20 22:14   ` David Elsing
2023-09-20 22:19     ` [bug#64446] [PATCH v2] " David Elsing
2023-10-11 17:11       ` bug#64446: " Ludovic Courtès
2023-07-26 16:22 ` [bug#64446] [PATCH] " Andreas Enge

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZMFFc6mBoKVRW8y8@jurong \
    --to=andreas@enge.fr \
    --cc=64446@debbugs.gnu.org \
    --cc=bavier@posteo.net \
    --cc=david.elsing@posteo.net \
    --cc=efraim@flashner.co.il \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).