bug#56900: phoronix-test-suite downloads nonfree software (+ question on collaboration with Parabola on this package)

2022-08-02 Thread Maxim Cournoyer
Hi Denis,

Denis 'GNUtoo' Carikli  writes:

> Hi,
>
> Thanks a lot for working on having a recent FSDG compliant versions of
> phoronix-test-suite.
>
> I've found a minor issue with phoronix-test-suite and I've a couple of
> questions to help synchronize between Parabola and Guix to prevent
> accidental installation of nonfree software.

Thanks for reaching out!

> Nonfree software downloaded by phoronix-test-suite:
> ---
> According to Debian, Linux has the following nonfree files:
>> Documentation/netlabel/draft-ietf-cipso-ipsecurity-01.txt
>> arch/powerpc/platforms/8xx/micropatch.c
>> drivers/media/usb/dvb-usb/af9005-script.h drivers/media/i2c/vs6624.c
>> drivers/net/appletalk/cops*
>> drivers/video/fbdev/nvidia
>> drivers/video/fbdev/riva
>
> The GNU project also mention that ("nonfree blobs are shipped with
> Linux, the kernel") in the list of distros that are not FSDG
> compliant[1].
>
> When running 'phoronix-test-suite test build-linux-kernel' (and
> selecting Yes(Y) to install the test), phoronix-test-suite downloads
> a tarball of Linux.
>
> So I assume that the tarball downloaded also have nonfree software like
> arch/powerpc/platforms/8xx/micropatch.c. So we also need to filter-out
> two tests (to keep being FSDG compliant):
>> pts/build-linux-kernel
>> pts/unpack-linux
>
> As I understand, the other build should be fine.

The test is probably marked as free (as in freedom) by the metadata
included in the test; we should rectify that in a source snippet and
open an issue upstream so that the correct metadata gets added.  Looking
a it, the unpack-firefox would fall in the same category I believe.

> Question on the package description and collaborating with Parabola:
> 
> If I run 'phoronix-test-suite list' on a fresh Guix installation, it
> lists nonfree software like BioShock Infinite, Hitman, which are games
> that have not been released under free licenses (according to
> Wikipedia).

I confirm:

$ phoronix-test-suite list-all-tests | grep -iE '(hitman|bioshock)'
pts/bioshock-infinite BioShock Infinite 
Graphics
pts/hitmanHITMAN
Graphics

And the reason why:

--8<---cut here---start->8---
$ grep License $(guix build 
phoronix-test-suite)/ob-cache/test-profiles/pts/hitman-1.0.1/test-definition.xml
Free

$ grep License 
/gnu/store/bdjf3g5c0xv0hhygag1rwjsvq11y7j1h-phoronix-test-suite-10.8.3/share/phoronix-test-suite/ob-cache/test-profiles/pts/bioshock-infinite-1.0.1/test-definition.xml
Free
--8<---cut here---end--->8---

We definitely should report these upstream, if they haven't been fixed
yet (there's a separate repository for the test metadata).

> If it didn't download any nonfree games, we could update the package
> description to explain how this FSDG compliance is done to not make
> users afraid and prevent invalid bug reports.

Good idea.

> I also saw there is a python script[2] that somehow is supposed to take
> care of FSDG compliance. How does it work in practice?

phoronix-test-suite comes with the test metadata so that it can at list
them offline upon first use.  Normally, it fetches updates of this
metadata but this gets disabled in our packaging of it, so only our
cleaned up offline metadata gets used.

So the Python script takes care of pruning nonfree tests from the
metadata, based on the metadata of the tests themselves (normally their
'License' field is set to 'Free' for free software).

I hope that helps clearing how it works.

Would you like to try preparing a snippet patch turning the above 'Free'
into 'Proprietary' or the likes so that the Python script can remove
them?  Otherwise I can look into it.

Thanks,

Maxim





bug#56900: phoronix-test-suite downloads nonfree software (+ question on collaboration with Parabola on this package)

2022-08-02 Thread Denis 'GNUtoo' Carikli
Hi,

Thanks a lot for working on having a recent FSDG compliant versions of
phoronix-test-suite.

I've found a minor issue with phoronix-test-suite and I've a couple of
questions to help synchronize between Parabola and Guix to prevent
accidental installation of nonfree software.

Nonfree software downloaded by phoronix-test-suite:
---
According to Debian, Linux has the following nonfree files:
> Documentation/netlabel/draft-ietf-cipso-ipsecurity-01.txt
> arch/powerpc/platforms/8xx/micropatch.c
> drivers/media/usb/dvb-usb/af9005-script.h drivers/media/i2c/vs6624.c
> drivers/net/appletalk/cops*
> drivers/video/fbdev/nvidia
> drivers/video/fbdev/riva

The GNU project also mention that ("nonfree blobs are shipped with
Linux, the kernel") in the list of distros that are not FSDG
compliant[1].

When running 'phoronix-test-suite test build-linux-kernel' (and
selecting Yes(Y) to install the test), phoronix-test-suite downloads
a tarball of Linux.

So I assume that the tarball downloaded also have nonfree software like
arch/powerpc/platforms/8xx/micropatch.c. So we also need to filter-out
two tests (to keep being FSDG compliant):
> pts/build-linux-kernel
> pts/unpack-linux

As I understand, the other build should be fine.

Question on the package description and collaborating with Parabola:

If I run 'phoronix-test-suite list' on a fresh Guix installation, it
lists nonfree software like BioShock Infinite, Hitman, which are games
that have not been released under free licenses (according to
Wikipedia).

I've installed Guix on a separate computer just for that (as it would
be negligence from my part to risk running nonfree software like that on
computers that are either hold keys to sign releases or build releases).

And there it seems that it runs hoses tests (I need to do that in a
terminal in a graphical interface like sway for instance) but I'm unsure
if it actually downloaded nonfree software or not as the tests seem to
fail and I didn't find the games being downloaded (but I only looked
rapidly).

If it didn't download any nonfree games, we could update the package
description to explain how this FSDG compliance is done to not make
users afraid and prevent invalid bug reports.

I also saw there is a python script[2] that somehow is supposed to take
care of FSDG compliance. How does it work in practice?

I'm also asking because I've also added phoronix-test-suite to Parabola
long time ago with a very different patch, and if for some reasons both
patches are incompatible, I don't want to end up in a situation where
users running Guix on top of Parabola would end up accidentally
installing nonfree software.

If that's the case it would be easier to fix the issue in Parabola
(just by updating phoronix-test-suite and taking care of informing the
users to delete their old ~/.phoronix-test-suite/) but I also need to
understand how the python script works first.

Once these questions are sorted out I'll try to install Parabola
instead of Guix on that test laptop, import my existing
~/.phoronix-test-suite/, install Guix on top, and install
phoronix-test-suite through Guix to see if the nonfree tests somehow
run something.

And I'll then report the result of test here and I'll then probably end
up updating in Parabola (and reusing your patch).

References:
---
[1]https://www.gnu.org/distros/common-distros.html
[2]gnu/packages/patches/phoronix-test-suite-fsdg.patch

Denis.


pgpODVyIlZJH1.pgp
Description: OpenPGP digital signature


bug#56893: rust-vergen inserts build timestamps

2022-08-02 Thread Maxime Devos

On 02-08-2022 20:41, Geert Stappers wrote:


Date: Tue, 2 Aug 2022 19:18:46 +0200, From: Maxime Devos

In Guix, I've noticed that rust-vergen embeds build timestamps. There is also
a work-around available: .
  


Thanks for reporting the FTBR.

Please update the workaround, so it looks more
like https://en.wikipedia.org/wiki/Diff#Unified_format
and can be absured by https://en.wikipedia.org/wiki/Patch_(Unix)


Just telling the filename that needs modification would be a great help.


Oops, I did not send the full work-around, here it is:


     (substitute* (find-files "." "\\.rs$")
       (("^extern crate chrono;") "extern crate chrono; use 
chrono::Utc; use chrono::TimeZone;")
       (("^use chrono::Utc;") "use chrono::Utc; use 
chrono::TimeZone;")

       (("\\bUtc::now\\(\\)") "Utc.timestamp(0, 0)"))

(Should hopefully be clearer now!)

The important thing here is replacing all instances of Utc::now() 
(across all Rust source files of rust-vergen) by Utc.timestamp(0, 0), 
the rest is just adding the required imports -- I have not made a list 
of all file names.  If you want a list, try "grep -rF Utc::now" or such.


I do not intend to update the workaround, it works fine in Guix and 
frankly porting it to whatever format Debian likes is Debian's concern, 
not Guix', I'm just sharing our workaround as a courtesy to another distro.


Greetings,
Maxime.



OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56895: rust-brotli-sys bundles (insecure!) brotli

2022-08-02 Thread Maxime Devos
Friendly reminder to the original patch author and committer (*) to 
check for bundling during review.


(*) 
https://git.savannah.gnu.org/cgit/guix.git/commit/?id=52cc16b38b1b01b2bb354ed5510120856de15d39


Greetings,
Maxime.


OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56895: rust-brotli-sys bundles (insecure!) brotli

2022-08-02 Thread Maxime Devos
I noticed rust-brotli-sys bundles brotli: 
.


The version it bundles is apparently insecure: 



As mentioned at , there 
have been multiple PR updating it to new PR but they were abandoned, so 
it appears we have to remove rust-brotli-sys entirely (in favour of 
rust-brotli?) or merge one of them (or better: unbundle) things on our own.


Greetings,
Maxime.



OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56893: rust-vergen inserts build timestamps, possible irreproducibility source

2022-08-02 Thread Maxime Devos
The following phase works around the issue for me (for antioxidant) -- 
this makes "guix build --check" pass (at least for antioxidant):


+    ;; TODO: SOURCE_DATE_EPOCH support would be nice.  Also maybe 
better fit for a snippet?

+    ;;
+    ;; Make the rust-vergen reproducible and avoid causing 
irreproducibility

+    ;; in dependents.
+    ("rust-vergen"
+ ,#~((add-after 'unpack 'remove-timestamp-irreproducibility
+       (lambda _
+     (substitute* (find-files "." "\\.rs$")
+       (("^extern crate chrono;") "extern crate chrono; use 
chrono::Utc; use chrono::TimeZone;")
+       (("^use chrono::Utc;") "use chrono::Utc; use 
chrono::TimeZone;")


Should also work for cargo-build-system, but untested.

Greetings,
Maxime



OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56893: rust-vergen inserts build timestamps, possible irreproducibility source

2022-08-02 Thread Maxime Devos
While fixing build failures in antioxidant, I noticed that rust-vergen 
is a potential source of irreproducibility -- the README.md contains the 
following:



## Documentation
[Documentation](https://docs.rs/vergen)

## Generate Compile Time Information
`vergen`, when used in conjunction with cargo [build scripts], will
generate environment variables to use with the `env!` macro. Below
is a list of the supported variables.

Key   | Sample Value
--|
VERGEN_BUILD_TIMESTAMP    |2018-08-09T15:15:57.282334589+00:000
VERGEN_BUILD_DATE |2018-08-09
VERGEN_SHA |75b390dc6c05a6a4aa2791cc7b3934591803bc22
VERGEN_SHA_SHORT  |75b390d
VERGEN_COMMIT_DATE    |2018-08-08
VERGEN_TARGET_TRIPLE  |x86_64-unknown-linux-gnu
VERGEN_SEMVER |v3.0.0
VERGEN_SEMVER_LIGHTWEIGHT |v3.0.0

I'll try patching out the timestamps with 1970-...

Greetings,
Maxime.




OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56799: (gnu services configuration) usage of *unspecified* is problematic

2022-08-02 Thread Maxim Cournoyer
Hi Ludovic,

Ludovic Courtès  writes:

[...]

>> Granted, few services outside of Jami probably made use of this, but it
>> was nevertheless a useful property.
>
> I don’t know of any.

I think mostly because few services make use of define-configuration.
While attempting to write a new VNC service, it quickly became a visible
annoyance:

--8<---cut here---start->8---
(define-configuration/no-serialization xvnc-configuration
  (xvnc
   (file-like tigervnc-server)
   "The package that provides the Xvnc binary.")
  (display-number
   (number 0)
   "The display number used by Xvnc.  You should set this to a number not
already used a Xorg server.")
  (geometry
   (string "1024x768")
   "The size of the desktop to be created.")
  (depth
   (color-depth 24)
   "The pixel depth in bits of the desktop to be created.  Accepted values are
16, 24 or 32.")
  (port
   maybe-port
   "The port on which to listen for connections from viewers.  When left
unspecified, it defaults to 5900 plus the display number.")
  (ipv4?
   (boolean #t)
   "Use IPv4 for incoming and outgoing connections.")
  (ipv6?
   (boolean #t)
   "Use IPv6 for incoming and outgoing connections.")
  (password-file
   maybe-string
   "The password file to use, if any.  Refer to vncpasswd(1) to learn how to
generate such a file.")
  (frame-rate
   (number 60)
   "The maximum number of updates per second sent to each client.")
  (security-types
   (security-types (list "None"))
   (format #f "The allowed security schemes to use for incoming connections.
The default is \"None\", which is safe given that Xvnc is configured to
authenticate the user via the display manager, and only for local connections.
Accepted values are any of the following: ~s" %security-types))
  (localhost?
   (boolean #t)
   "Only allow connections from the same machine.  It is set to #true by
default for security, which means SSH or another secure means should be used
to expose the remote port.")
  (log-level
   (log-level 30)
   "The log level, a number between 0 and 100, 100 meaning most verbose
output.  The log messages are output to syslog.")
  (extra-options
   (strings '())
   "This can be used to provide extra Xvnc options not exposed via this
 record."))

[...]

(define (xvnc-shepherd-service config)
  "Return a  for Xvnc with CONFIG."
  ;; XXX: Ensure all the *unspecified* values are handled outside of gexps, as
  ;; they are not valid gexp input (they are not self-quoting/serializable).
  ;; This would otherwise cause problem during 'guix deploy'.
  (let* ((display-number (xvnc-configuration-display-number config))
 (port (if (unspecified? (xvnc-configuration-port config))
   #f
   (xvnc-configuration-port config)))
 (port* (or port (+ 5900 display-number)))
 (inaddr (if (xvnc-configuration-localhost? config)
 INADDR_LOOPBACK
 INADDR_ANY))
 (in6addr (if (xvnc-configuration-localhost? config)
  IN6ADDR_LOOPBACK
  IN6ADDR_ANY))
 (ipv4-socket (and (xvnc-configuration-ipv4? config)
   (make-socket-address AF_INET inaddr
port*)))
 (ipv6-socket (and (xvnc-configuration-ipv6? config)
   (make-socket-address AF_INET6 in6addr
port*
(shepherd-service
 (provision '(xvnc vncserver))
 (documentation "Run the Xvnc server.")
 (requirement '(networking syslogd))
 (start #~(make-inetd-constructor
   #$(xvnc-configuration->command-line-arguments config)
   `(,@(if #$ipv4-socket
   (list (endpoint #$ipv4-socket))
   '())
 ,@(if #$ipv6-socket
   (list (endpoint #$ipv6-socket))
   '()
 (stop #~(make-inetd-destructor)
--8<---cut here---end--->8---

As you can see, the *unspecified* values need to be carefully massaged
out before starting to assemble the G-exp, as they aren't valid G-Exp
inputs.

> Having spent time reviewing the original change that Attila proposed and
> then chiming in on this issue, I would have hoped for a longer
> discussion before enacting the change in
> a2b89a3319dc1d621c546855f578acae5baaf6da.

OK.

> In addition to these issues around the process, I think we should strive
> for more stability.  One of the reasons it took time to review
>  is that interface changes are a
> commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
> introduces a second interface change for reasons that are unclear to me
> (if the conclusion had been to revert, I’d have favored an actual revert
> rather than introducing 'unset).

I like to think of *unspecified* or 'unset as an uninteresting
implementation detail that 

bug#56786: git-minimal fails to build (non-deterministic)

2022-08-02 Thread Maxime Devos
The log that was sent was for a successful build (maybe they retried the 
build?), seems like we will have to wait for another report.


Greetings,
Maxime.



OpenPGP_0x49E3EE22191725EE.asc
Description: OpenPGP public key


OpenPGP_signature
Description: OpenPGP digital signature


bug#56799: (gnu services configuration) usage of *unspecified* is problematic

2022-08-02 Thread bokr
Hi,

On +2022-08-02 09:31:14 +0200, Ludovic Courtès wrote:
> Hi,
> 
> Maxim Cournoyer  skribis:
> 
> > Ludovic Courtès  writes:
> >
> >> Hi!
> >>
> >> Maxim Cournoyer  skribis:
> >>
> >>> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
> >>> define-configuration machinery in (gnu services configuration) uses
> >>> *unspecified* instead of 'disabled for an unspecified field value.
> >>
> >> As Attila wrote, the rationale as discussed in
> >>  was to specifically use a “special”
> >> value without a read syntax in lieu of a symbol like 'disabled.
> >>
> >>> While this is indeed an improvement in readability, it introduces an
> >>> extra complication: because this new value is not self-quoting, it
> >>> cannot be used as is in G-Exps, and values using it must be carefully
> >>> expanded outside the gexp context, which is error prone.
> >>
> >> Could you give a simple example of how this can happen?
> >>
> >> In my experience, one would use ‘define-maybe’ and appropriate field
> >> serializers such that *unspecified* never goes through.  Previously
> >> you’d check for (eq? x 'disabled) and now you just check for
> >> (unspecified? x).
> >
> > Yes, I understand that.  What changed is that previously you could have
> > the configuration serialized and used on the service side, which is what
> > using *unspecified* made impossible.
> 
> Do you have an example?  Even on the service side, I imagine one could
> check for ‘unspecified?’ just like one would check for 'disabled, no?
> 
> > Granted, few services outside of Jami probably made use of this, but it
> > was nevertheless a useful property.
> 
> I don’t know of any.
> 
> Having spent time reviewing the original change that Attila proposed and
> then chiming in on this issue, I would have hoped for a longer
> discussion before enacting the change in
> a2b89a3319dc1d621c546855f578acae5baaf6da.
> 
> In addition to these issues around the process, I think we should strive
> for more stability.  One of the reasons it took time to review
>  is that interface changes are a
> commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
> introduces a second interface change for reasons that are unclear to me
> (if the conclusion had been to revert, I’d have favored an actual revert
> rather than introducing 'unset).
> 
> How should we move forward?
>

My 2¢ :

Maybe separate commmit churn more formally into a
release candidate series like Linus does for linux kernel,
and have a long term stable guix only get what is agreed
with solid consensus?

AND, importantly: where issues involve subtleties
of abstract entities vs their concrete representations,
make sure this is clearly documented in the commit rationale,
e.g., maybe using denottional semantics[1] like r5rs ?

[1]:  

:)

> Thanks,
> Ludo’.
>
--
Regards,
Bengt Richter
OT PS: Has Boot-to-guile been updated by anyone?
Kudos for the original! :) A RISCV qemu image? :)





bug#56799: (gnu services configuration) usage of *unspecified* is problematic

2022-08-02 Thread Ludovic Courtès
Hi,

Maxim Cournoyer  skribis:

> Ludovic Courtès  writes:
>
>> Hi!
>>
>> Maxim Cournoyer  skribis:
>>
>>> Since commit 8cb1a49a3998c39f315a4199b7d4a121a6d66449, the
>>> define-configuration machinery in (gnu services configuration) uses
>>> *unspecified* instead of 'disabled for an unspecified field value.
>>
>> As Attila wrote, the rationale as discussed in
>>  was to specifically use a “special”
>> value without a read syntax in lieu of a symbol like 'disabled.
>>
>>> While this is indeed an improvement in readability, it introduces an
>>> extra complication: because this new value is not self-quoting, it
>>> cannot be used as is in G-Exps, and values using it must be carefully
>>> expanded outside the gexp context, which is error prone.
>>
>> Could you give a simple example of how this can happen?
>>
>> In my experience, one would use ‘define-maybe’ and appropriate field
>> serializers such that *unspecified* never goes through.  Previously
>> you’d check for (eq? x 'disabled) and now you just check for
>> (unspecified? x).
>
> Yes, I understand that.  What changed is that previously you could have
> the configuration serialized and used on the service side, which is what
> using *unspecified* made impossible.

Do you have an example?  Even on the service side, I imagine one could
check for ‘unspecified?’ just like one would check for 'disabled, no?

> Granted, few services outside of Jami probably made use of this, but it
> was nevertheless a useful property.

I don’t know of any.

Having spent time reviewing the original change that Attila proposed and
then chiming in on this issue, I would have hoped for a longer
discussion before enacting the change in
a2b89a3319dc1d621c546855f578acae5baaf6da.

In addition to these issues around the process, I think we should strive
for more stability.  One of the reasons it took time to review
 is that interface changes are a
commitment.  Now commit a2b89a3319dc1d621c546855f578acae5baaf6da
introduces a second interface change for reasons that are unclear to me
(if the conclusion had been to revert, I’d have favored an actual revert
rather than introducing 'unset).

How should we move forward?

Thanks,
Ludo’.