GNU bug report logs - #52283
[PATCH 00/10] Tuning packages for CPU micro-architectures

Previous Next

Full log

View this message in rfc822 format

From: Thiago Jung Bauermann <bauermann <at> kolabnow.com>
To: 52283 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludovic.courtes <at> inria.fr>, Ludovic Courtès <ludo <at> gnu.org>
Subject: [bug#52283] [PATCH 02/10] transformations: Add '--tune'.
Date: Mon, 06 Dec 2021 20:18:51 -0300

Hello Ludo,

Awesome series! I only have comments about this patch, and then only minor 
ones:

Em sábado, 4 de dezembro de 2021, às 17:49:16 -03, Ludovic Courtès 
escreveu:
> +Tuned packages are @emph{grafted} onto packages that depend on them
> +(@pxref{Security Updates, grafts}).  Thus, using @option{--no-grafts}
> +annihilates the effect of @option{--tune}.

Perhaps this is because English isn’t my first language, but annihilation 
seems like a violent and dramatic effect in a package transformation. :-)

Perhaps reword as “cancels”, “invalidates” or "nullifies"?

> +(define (tuned-package p micro-architecture)
> +  "Return package P tuned for MICRO-ARCHITECTURE."
> +  (define compiler
> +    (tuning-compiler micro-architecture))
> +
> +  (package
> +    (inherit p)
> +    (native-inputs
> +     ;; Arrange so that COMPILER comes first in $PATH.
> +     `(("tuning-compiler" ,compiler)
> +       ,@(package-native-inputs p)))
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments p)
> +       ((#:tests? _ #f) #f)))

Perhaps I’m reading this wrong, but it looks like tuned packages don’t run 
their testsuites? If so, this is a surprising side-effect and thus it would 
be nice to have it mentioned in the manual, possibly also in a comment 
here. It would be nice to also mention the rationale for disabling the 
tests (not sure whether only in a comment here or if in the manual as 
well). I assume it’s for convenience, but I’m not sure.

-- 
Thanks,
Thiago

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.