Subject: [bug#47399] [PATCH] Add Google's Highway library.

From: Maxime Devos <[email-masked]>
Date: Fri, 26 Mar 2021 10:17:24 +0100

On Fri, 2021-03-26 at 00:54 +0000, Isaac Young via Guix-patches via wrote:
> Hello, 
> 
> I would like to add Google's highway library (https://github.com/google/highway) to guix officially. Any feedback will be greately apperciated as this is my first public package.

A commit message is missing from the patch.  E.g. something like

commit b09967bc587dd61c638da3d6fb5bbda7e87ed85c
Author: Raghav Gururajan <[email-masked]>
Date:   Sun Mar 7 01:47:19 2021 -0500

    gnu: Add psi-plus.
    
    * gnu/packages/messaging.scm (psi-plus): New variable.

(See 16.1 ‘Submitting Patches’ in the manual.)

You should add a ‘Copyright YEAR your-name’ as well at the top of the file.
There is no need to add ‘#:tests? #t’ as tests are enabled by default.

What's HWY_SYSTEM_GTEST=ON for?  Maybe add a comment

;; Prevent the configuration script from trying to download googletest
;; from the network.

or something like that.  Run "./pre-inst-env guix lint google-highway"
if you haven't already.

The "googletest" input does not make much sense when cross-compiling;
maybe move it to native-inputs with something like

  (native-inputs
   ,@(if (%current-target-system)
         '()
         ;; only used by the test suite
         `(("googletest" ,googletest))))

(See 8.2.1 ‘‘package’ reference’.)

(Admittedly, we don't focus much on making packages cross-compilable,
except for packages like gcc, bash, binutils ..., so no big problem
if this package isn't cross-compilable.)

Assuming you're on a system different from "aarch64-linux-gnu",
you can test cross-compilability with
"./pre-inst-env guix build --target=aarch64-linux-gnu google-highway".
(See 9.3.1 ‘Additional Build Options’)

Otherwise no comments, assuming it builds!

> Thank you,
> Isaac Young

Greetings,
Maxime.