date:20210422

Ludovic Courtès  writes:

> Hi!
>
> (“Sorry for the long delay” is officially my motto at this point.)
>
> Christopher Baines  skribis:
>
>> This has been on my mind for a while, as I wonder what effect it has on
>> users fetching substitues.
>>
>> The narinfo caching as I understand it works as follows:
>>
>>  Default success TTL => 36 hours
>>  Negative TTL=> 1 hour
>>  Transient error TTL => 10 minutes
>>
>> I'm ignoring the success TTL, I'm just interested in the negative and
>> transient error values. Negative means that when a server says it
>> doesn't have an output, that response will be cached for an
>> hour. Transient errors are for other HTTP response codes, like 504.
>
> You’re looking at the default TTLs, which are not the actual TTLs.
> Specifically, servers can include a ‘Cache-Control’ header in their
> reply specifying the TTL of their choice, and ‘guix substitute’ honors
> that:
>
>   https://git.savannah.gnu.org/cgit/guix.git/tree/guix/substitutes.scm#n200
>   
> https://git.savannah.gnu.org/cgit/guix.git/tree/guix/scripts/publish.scm#n371
>
> ‘guix publish’ returns 404 with a TTL of 5mn when the requested item is
> in store but needs to be “baked”.
>
> However, ‘guix publish’ does not set ‘Cache-Control’ when the request
> item is not in store.  In that case, clients use ‘%narinfo-negative-ttl’
> (1h).

You're right that the negative ttl is just a default, so it's possible
to override the default behaviour in the success and negative lookup
cases, but I don't believe the Cache-Control header is used for
transient errors.

>> I had a look through the Git history, caching negative lookups has been
>> a thing for a while. Caching transient errors was added, but I couldn't
>> see why.
>
> Transient error caching was most likely added in the days of
> hydra.gnu.org, that VM that was extremely slow.  When overloaded, you’d
> get 500 or similar, and at that point it was safer for clients to wait
> and come back later, possibly much later.  :-)
>
>> Personally I don't see a reason to keep either behaviours?
>
> The main arguments for these negative TTLs are:
>
>   1. Reducing server load: if the server doesn’t have libreoffice, don’t
>  come back asking every 10s, it’s prolly useless.  You could easily
>  have “GET storms” for libreoffice if clients don’t restrain
>  themselves.
>
>   2. Improving client performance: don’t GET things that are likely to
>  fail.

As you say, for the negative TTL, the question here is really what's the
best default value, if a server isn't specifying one.

Given that most narinfo requests precede a build for that thing if the
response is negative, I have my doubts about those two arguments
above. This is assuming the most common case is users asking guix to
install and upgrade things.

If a user gets a negative response, they'll just build it instead and
not check for that narinfo again. Even if they cancel that build when
they realise they don't want to build libreoffice, they'll wait a bit
anyway before retrying.

> Now, the penalty it imposes is annoying.  I’ve sometimes found myself
> working around it, too (because I knew the server was going to have the
> store item sooner than 1h).
>
> Rather than removing it entirely, I can think of these options:
>
>   1. Reduce the default negative timeouts.

I think reducing it is good, as you say, it's possible to override the
default from the server side. Just in case someone wants caching
behaviour, it might be worth keeping that functionality at least.

>   2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
>  send a ‘Cache-Control’ header with the chosen TTL on 404.  That
>  way, if the server operator doesn’t mind extra load, they can run
>  “guix publish --negative-ttl=0”.

That sounds sensible. The Guix Build Coordinator doesn't do any serving,
that's left to something else like nginx. For the deployments I maintain
though, I don't think I'm setting the relevant headers, but I'll look at
changing that.

Going back to the %narinfo-transient-error-ttl, if I'm correct in saying
that it's not possible to override that, maybe that should also use the
relevant header value if set?

signature.asc
Description: PGP signature

Re: Add a way to disable serialization support to (guix services configuration)

Hi,

Maxim Cournoyer  skribis:

>> +(define-syntax-rule (without-field-serialization definition)
>> +  (syntax-parameterize ((configuration-field-serialization?
>> + (identifier-syntax #f)))
>> +definition
>> +#t))
>> +
>> +(without-field-serialization
>> +  (define-configuration foo
>> +(bar (integer 123) "doc")))

In hindsight, I find this syntax quite inelegant and suboptimal.

Wouldn’t it be nicer to write:

  (define-configuration foo
(bar (integer 123) "doc" no-serializer)
(baz (string "") "doc"))

where ‘bar’ wouldn’t have a serializer and ‘baz’ would?

It’s also probably easier to implement correctly.

Ludo’.

Re: Narinfo negative and transient error caching

BTW, one thing that would be interesting too is to return 404 with a
long ‘Cache-Control’ validity when the requested store item is among the
cached failures.

We could also add an extra response header to explicitly communicate
that the store item is known to fail to build.

Ludo’.

Re: Narinfo negative and transient error caching

Hi!

(“Sorry for the long delay” is officially my motto at this point.)

Christopher Baines  skribis:

> This has been on my mind for a while, as I wonder what effect it has on
> users fetching substitues.
>
> The narinfo caching as I understand it works as follows:
>
>  Default success TTL => 36 hours
>  Negative TTL=> 1 hour
>  Transient error TTL => 10 minutes
>
> I'm ignoring the success TTL, I'm just interested in the negative and
> transient error values. Negative means that when a server says it
> doesn't have an output, that response will be cached for an
> hour. Transient errors are for other HTTP response codes, like 504.

You’re looking at the default TTLs, which are not the actual TTLs.
Specifically, servers can include a ‘Cache-Control’ header in their
reply specifying the TTL of their choice, and ‘guix substitute’ honors
that:

  https://git.savannah.gnu.org/cgit/guix.git/tree/guix/substitutes.scm#n200
  https://git.savannah.gnu.org/cgit/guix.git/tree/guix/scripts/publish.scm#n371

‘guix publish’ returns 404 with a TTL of 5mn when the requested item is
in store but needs to be “baked”.

However, ‘guix publish’ does not set ‘Cache-Control’ when the request
item is not in store.  In that case, clients use ‘%narinfo-negative-ttl’
(1h).

> I had a look through the Git history, caching negative lookups has been
> a thing for a while. Caching transient errors was added, but I couldn't
> see why.

Transient error caching was most likely added in the days of
hydra.gnu.org, that VM that was extremely slow.  When overloaded, you’d
get 500 or similar, and at that point it was safer for clients to wait
and come back later, possibly much later.  :-)

> Personally I don't see a reason to keep either behaviours?

The main arguments for these negative TTLs are:

  1. Reducing server load: if the server doesn’t have libreoffice, don’t
 come back asking every 10s, it’s prolly useless.  You could easily
 have “GET storms” for libreoffice if clients don’t restrain
 themselves.

  2. Improving client performance: don’t GET things that are likely to
 fail.

Now, the penalty it imposes is annoying.  I’ve sometimes found myself
working around it, too (because I knew the server was going to have the
store item sooner than 1h).

Rather than removing it entirely, I can think of these options:

  1. Reduce the default negative timeouts.

  2. Add an option to ‘guix publish’ (and to the Coordinator?) so they
 send a ‘Cache-Control’ header with the chosen TTL on 404.  That
 way, if the server operator doesn’t mind extra load, they can run
 “guix publish --negative-ttl=0”.

WDYT?  Does that make any sense?

Ludo’.

Re: Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes)

Hi Guix!

Thanks Mark for raising these issues. I definitely share your concerns,
specifically regarding the two commits you mentioned and how they
misleadingly have undesirable consequences:

https://git.savannah.gnu.org/cgit/guix.git/commit/?h=wip-gnome=d975ed975456a2c8e855eb024b5487c4c460684a

https://git.savannah.gnu.org/cgit/guix.git/commit/?h=wip-gnome=f5fc3c609e2f38ca1c0523deadb9f77d838fbf32

I believe all the parties involved behaved in good faith and I’m more
interested in (1) fixing these two mistakes, and (2) making sure this
doesn’t happen again in the future.

Regarding #1, I see 宋文武 (iyzsong) proposed a patch that reinstates
the Cairo security fixes, so it seems we’re on the right track.

Raghav, could you post a patch reinstating the gdk-pixbuf security fix
removed in f5fc3c609e2f38ca1c0523deadb9f77d838fbf32? If that commit is
only on ‘wip-gnome’, can you simply drop it? (The convention is that a
‘wip-’ branch may be rebased at will by the person responsible for it.)

Could you also check ‘wip-gnome’ and ‘core-updates’ for similar
“cosmetic changes” commits likely to be controversial?

Regarding #2, everyone please keep in mind the commit rules¹. We’re all
pouring hours of our time into this. It’s a social project before being
a technical one, so it’s crucial to not step on each other’s toes.

As per these rules, ‘wip-gnome’ will have to go through review as usual.
As always, it’s best if you can submit it for review in small chunks.
Note that review has to happen via guix-patc...@gnu.org so everyone in
the project can see it and has a chance to chime in. You can have
pre-review/mentoring elsewhere if you want (it’s great if you can have
that), but it “doesn’t count” from the project’s viewpoint.

I think committers and particularly vouchers should spend more time
reviewing and mentoring newcomers.

Regarding commit logs, we only add a ‘Signed-off-by’ line when applying
someone else’s patches; let’s keep following that rule, for clarity.

For the subject line: as 宋文武 wrote, as a rule of thumb, if you cannot
summarize the change in the subject line, that probably means the patch
should be split into smaller logical units. More generally, never
bundle together unrelated changes in the same commit, as per:

https://guix.gnu.org/manual/en/html_node/Submitting-Patches.html

Here are additional rules I try to follow for the commit message and
that I’d recommend following:

1. Never write “cosmetic changes” as the summary: it’s too vague.
Instead, be more specific: “reindent foo.scm”, “remove unused
module imports”, whatever.

2. Never write “Fix X”. Instead describe the fix: “Avoid
non-top-level 'use-modules'”, “Remove file that no longer exists”,
“Honor proxy settings”, “Disable tests when building on i586-gnu”,
etc. In all these examples I could have written “Fix …”, but
fellow hackers would have had to look at the diff to understand
what’s going on.

A long message!

Let’s calm down and focus the way forward: fixing the security issues
that were reported, checking whether similar ones are lurking, and
improving our practices. If you feel the need for it, feel free to
propose improvements to the “Commit Access” and “Submitting Patches”
sections, too!

Thanks in advance. :-)

Ludo’.

¹ https://guix.gnu.org/manual/en/html_node/Commit-Access.html

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread Raghav Gururajan


Hi Leo!


Raghav and Léo, is wip-gnome based on core-updates?


It was based on core-updates, but I recently re-created wip-gnome based 
on master.


Regards,
RG.



OpenPGP_signature
Description: OpenPGP digital signature

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread Raghav Gururajan


Hi Mark!


Thank you for these links.  From the IRC log cited above, it now appears
that Léo Le Bouter  bears primary responsibility
for these mistakes.  In particular, according to the IRC
logs, Léo wrote:

raghavgururajan: the main issues on the rebasing were about
   security fixes on cairo, gdk-pixbuf and glib
I modified the cosmetic commits to remove the graft and
   patches etc.

   
Yes, Leo made these changes. But I also have to acknowledge that it was 
part of my responsibility to question any changes. My apologies for 
that. Leo was occupied with many stuff on those days (with CVE bug 
hunting etc.). Its understandable that it was a honest mistake.



Nonetheless, you (Raghav) also bear some responsibility for digitally
signing and pushing these misleading commits to the 'wip-gnome' branch.
At least one of the problems (the misleading summary line) should have
been obvious from a cursory glance at the commit log.


Yes, my apologies. I will merge fix commits from core-updates to wip-gnome.

Regards,
RG.



OpenPGP_signature
Description: OpenPGP digital signature

Re: Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes)

Léo Le Bouter  writes:

> On Thu, 2021-04-22 at 13:40 -0400, Mark H Weaver wrote:
>> This commit was digitally signed and pushed to the 'wip-gnome' branch
>> by
>> Raghav, but it's also "Signed-off-by: Léo Le Bouter", so I'm not sure
>> who bears primary responsibility for this one.
>
> It seems you are more focused and spend more time sending accusations
> here than collaboratively working to improve GNU Guix. I don't feel
> like that's something great to do at all.

I feel an obligation to protect our users, and among other things that
means calling attention to Guix committers that are doing things like
pushing commits with misleading commit logs (which evade proper review)
and pushing "cosmetic changes" that remove security fixes.

I've given evidence to back up these claims, evidence that anyone who
cares to look in our git repository can plainly see.  If you believe my
conclusions are erroneous, please make your case.

   Mark

-- 
Support Richard Stallman against the vicious misinformation campaign
against him and the FSF.  See  for more.

Re: Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes)

2021-04-22 Thread Ricardo Wurmus




Léo,


On Thu, 2021-04-22 at 13:40 -0400, Mark H Weaver wrote:
This commit was digitally signed and pushed to the 'wip-gnome' 
branch

by
Raghav, but it's also "Signed-off-by: Léo Le Bouter", so I'm 
not sure

who bears primary responsibility for this one.


It seems you are more focused and spend more time sending 
accusations
here than collaboratively working to improve GNU Guix. I don't 
feel

like that's something great to do at all.


Please try to deescalate.

While I disagree with Mark’s earlier style and harsh tone towards 
Raghav (who has been trying to get more of his older commits that 
are relevant for a speedy upgrade of Gnome in shape), it seems 
obvious now that Mark attempts to understand what signing off on 
the changed commit means in this case – what motivated the 
expansion of the scope of the commit, what resulted in not 
declaring all of the significant changes.


I do think it is reasonable to expect an explanation, so that we 
can collectively address the potential issue (patches removed 
without upgrade), and prevent it from happening again.


Escalating the situation by insinuating bad faith does not serve 
any constructive purpose.  Obstructing the investigation into what 
happened will only make it harder to prevent it from happening 
again.  It is in our shared interest to prevent problems like 
this.  Also note that Guix has a pretty good track record when it 
comes to clear commits, so it is not unusual for long term 
contributors to be at the very least confused by what has happened 
here.


--
Ricardo

Re: A "cosmetic changes" commit that removes security fixes

Hi Léo,

Léo Le Bouter  writes:

> I don't share your analysis, the security fixes werent stripped because
> glib/cairo was also updated to latest version in subsequent commits
> which were pushed all at once.

'glib' was updated, but 'cairo' wasn't, presumably because there's no
newer stable release of 'cairo' to update to.

> Careful review was done, and that's why I signed-off and GPG-signed the
> commits. Nobody was put at risk by these commits and no security fixes
> were stripped.

Those are bold claims, given the contents of our git repository.

Here's Raghav's commit on the 'core-updates' branch, which bears your
digital signature (according to my 'git' client), where the security
fixes for CVE-2018-19876 and CVE-2020-35492 were removed, in a commit
whose summary line is "gnu: cairo: Make some cosmetic changes":

  
https://git.sv.gnu.org/cgit/guix.git/commit/?h=core-updates=f94cdc86f644984ca83164d40b17e7eed6e22091

I have two questions for you:

(1) Do you deny that you digitally signed that commit?
(2) Do you deny that there's anything wrong with that commit?

 Thanks,
   Mark

-- 
Support Richard Stallman against the vicious misinformation campaign
against him and the FSF.  See  for more.

Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.


Luciana Lima Brito  writes:

> On Thu, 22 Apr 2021 21:08:08 +0100
> Christopher Baines  wrote:
>
>> I'm not quite sure what you mean by empty fields in the JSON here?
>
> I mean now it appears like this in the json
>
> hash-alg: {}
>
> Before, it was entirely omitted.

Right, OK. I'd call that an empty object.

>> It sounds like you're roughly on the right track, do share what
>> changes you're making and then I can have a look.
>
> This is the part I've changed on comparison.scm in the function
> derivation-outputs-differences-data:
>
>  (map (match-lambda
>  ((output-name path hash-algorithm hash recursive
>derivation_ids)
>   (let ((parsed-derivation-ids
>  (map string->number
>   (parse-postgresql-array-string derivation_ids
> (list output-name
>   path
>   (if (string? hash-algorithm)
>   hash-algorithm
>   '())
>   (if (string? hash)
>   hash
>   '())
>   (string=? recursive "t")
>   (append (if (memq base-derivation-id
> parsed-derivation-ids)
>   '(base)
>   '())
>   (if (memq target-derivation-id
> parsed-derivation-ids)
>   '(target)
>   '()))
>(exec-query conn query))
>
> Basically, I moved the hash-algorithm, hash, and recursive tests from
> controller.scm here. I spent more time familiarizing with this part of
> the code and the dataset. I don't have anything ready for the idea I
> told you in the last email because I'm not so sure if that is the way
> to go, but I can make some experiments in this regard.

The bit for the recursive field looks fine.

I'd suggest avoiding '() as the value for hash and hash-algorithm when
they're NULL in the database. One option here that I've used in some
places is to return a alist rather than a list. This can simplify JSON
output since it's often a alist that's desired, and can avoid breaking
matches when they're matching on the list.

Does that make sense?


signature.asc
Description: PGP signature

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread Leo Prikler

Am Donnerstag, den 22.04.2021, 22:01 +0200 schrieb Léo Le Bouter:
> On Thu, 2021-04-22 at 00:08 -0400, Mark H Weaver wrote:
> > Hi Raghav,
> > 
> > Raghav Gururajan  writes:
> > 
> > > > Those commits on 'core-updates' were digitally signed by Léo Le
> > > > Bouter
> > > >  and have the same problems: they remove
> > > > security
> > > > fixes, and yet the summary lines indicate that only "cosmetic
> > > > changes"
> > > > were made.
> > > 
> > > Yeah, the commit title didn't mention the change but the commit
> > > message did.
> > 
> > I'm sorry, but that won't do.  There are at least three things
> > wrong
> > with these commits:
> > 
> > (1) The summary lines were misleading, because they implied that no
> > functional changes were made.
> > 
> > (2) The commit messages were misleading, because they failed to
> > mention
> > that security holes which had previously been fixed were now
> > being
> > re-introduced.  That wasn't at all obvious.
> > 
> > Commits like these, which remove patches that had fixed
> > security
> > flaws, are fairly common: someone casually looking over the
> > commit
> > log might assume that the patches could be safely removed
> > because
> > a
> > version update was done at the same time, rendering those
> > patches
> > obsolete.
> > 
> > (3) Although your 'glib' commit was immediately followed by a
> > 'glib'
> > update, rendering it harmless, your misleading 'cairo' commit
> > left
> > 'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our
> > 'core-updates' and 'wip-gnome' branches.  Those will need to be
> > fixed now.
> > 
> > Léo Le Bouter  is also culpable here, because
> > he
> > digitally signed the misleading 'cairo' commit that's on our
> > 'core-updates' branch, which re-introduced CVE-2018-19876 and
> > CVE-2020-35492.
> > 
> > --8<---cut here---start->8---
> > commit f94cdc86f644984ca83164d40b17e7eed6e22091
> > gpg: Signature made Fri 26 Mar 2021 05:13:57 PM EDT
> > gpg:using RSA key
> > 148BCB8BD80BFB16B1DE0E9145A8B1E86BCD10A6
> > gpg: Good signature from "Léo Le Bouter "
> > [unknown]
> > gpg: WARNING: This key is not certified with a trusted signature!
> > gpg:  There is no indication that the signature belongs to
> > the owner.
> > Primary key fingerprint: 148B CB8B D80B FB16 B1DE  0E91 45A8 B1E8
> > 6BCD 10A6
> > Author: Raghav Gururajan 
> > Date:   Fri Dec 4 00:48:43 2020 -0500
> > 
> > gnu: cairo: Make some cosmetic changes.
> > 
> > * gnu/packages/patches/cairo-CVE-2018-19876.patch,
> > gnu/packages/patches/cairo-CVE-2020-35492.patch: Remove
> > patches.
> > * gnu/local.mk (dist_patch_DATA): Unregister them.
> > * gnu/packages/gtk.scm (cairo): Make some cosmetic changes.
> > [replacement]: Remove.
> > (cairo/fixed): Remove.
> > 
> > Signed-off-by: Léo Le Bouter 
> > --8<---cut here---end--->8---
> > 
> > https://git.sv.gnu.org/cgit/guix.git/commit/?h=core-updates=f94cdc86f644984ca83164d40b17e7eed6e22091
> > 
> > Even the most superficial skimming of this commit should have
> > immediately raised red flags, because the summary line is clearly
> > inaccurate.  It shows a lack of careful review, to put it mildly.
> > 
> >   Mark
> 
> Hello Mark,
> 
> I don't share your analysis, the security fixes werent stripped
> because
> glib/cairo was also updated to latest version in subsequent commits
> which were pushed all at once.
This may be the case for glib, but which commit pushes cairo to a
version, in which those security fixes are applied?

Re: A "cosmetic changes" commit that removes security fixes


Léo Le Bouter  writes:

> On Thu, 2021-04-22 at 00:08 -0400, Mark H Weaver wrote:
>> Hi Raghav,
>> 
>> Raghav Gururajan  writes:
>> 
>> > > Those commits on 'core-updates' were digitally signed by Léo Le
>> > > Bouter
>> > >  and have the same problems: they remove
>> > > security
>> > > fixes, and yet the summary lines indicate that only "cosmetic
>> > > changes"
>> > > were made.
>> > 
>> > Yeah, the commit title didn't mention the change but the commit
>> > message did.
>> 
>> I'm sorry, but that won't do.  There are at least three things wrong
>> with these commits:
>> 
>> (1) The summary lines were misleading, because they implied that no
>> functional changes were made.
>> 
>> (2) The commit messages were misleading, because they failed to
>> mention
>> that security holes which had previously been fixed were now
>> being
>> re-introduced.  That wasn't at all obvious.
>> 
>> Commits like these, which remove patches that had fixed security
>> flaws, are fairly common: someone casually looking over the
>> commit
>> log might assume that the patches could be safely removed because
>> a
>> version update was done at the same time, rendering those patches
>> obsolete.
>> 
>> (3) Although your 'glib' commit was immediately followed by a 'glib'
>> update, rendering it harmless, your misleading 'cairo' commit
>> left
>> 'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our
>> 'core-updates' and 'wip-gnome' branches.  Those will need to be
>> fixed now.
>> 
>> Léo Le Bouter  is also culpable here, because he
>> digitally signed the misleading 'cairo' commit that's on our
>> 'core-updates' branch, which re-introduced CVE-2018-19876 and
>> CVE-2020-35492.
>> 
>> --8<---cut here---start->8---
>> commit f94cdc86f644984ca83164d40b17e7eed6e22091
>> gpg: Signature made Fri 26 Mar 2021 05:13:57 PM EDT
>> gpg:using RSA key
>> 148BCB8BD80BFB16B1DE0E9145A8B1E86BCD10A6
>> gpg: Good signature from "Léo Le Bouter "
>> [unknown]
>> gpg: WARNING: This key is not certified with a trusted signature!
>> gpg:  There is no indication that the signature belongs to
>> the owner.
>> Primary key fingerprint: 148B CB8B D80B FB16 B1DE  0E91 45A8 B1E8
>> 6BCD 10A6
>> Author: Raghav Gururajan 
>> Date:   Fri Dec 4 00:48:43 2020 -0500
>> 
>> gnu: cairo: Make some cosmetic changes.
>> 
>> * gnu/packages/patches/cairo-CVE-2018-19876.patch,
>> gnu/packages/patches/cairo-CVE-2020-35492.patch: Remove patches.
>> * gnu/local.mk (dist_patch_DATA): Unregister them.
>> * gnu/packages/gtk.scm (cairo): Make some cosmetic changes.
>> [replacement]: Remove.
>> (cairo/fixed): Remove.
>> 
>> Signed-off-by: Léo Le Bouter 
>> --8<---cut here---end--->8---
>> 
>> https://git.sv.gnu.org/cgit/guix.git/commit/?h=core-updates=f94cdc86f644984ca83164d40b17e7eed6e22091
>> 
>> Even the most superficial skimming of this commit should have
>> immediately raised red flags, because the summary line is clearly
>> inaccurate.  It shows a lack of careful review, to put it mildly.
>> 
>>   Mark
>
> Hello Mark,
>
> I don't share your analysis, the security fixes werent stripped because
> glib/cairo was also updated to latest version in subsequent commits
> which were pushed all at once.
>
> Careful review was done, and that's why I signed-off and GPG-signed the
> commits. Nobody was put at risk by these commits and no security fixes
> were stripped.

I think the guidance is that commits should include one set of related
changes, so if the patches/replacement can be removed because the
package is being updated, those related changes should be in the same
commit. If there are other unrelated changes, they can go in other
commits.

Especially if the commits are being pushed at the same time, it's worth
making sure this happens, so that it's easier to review and look at the
changes in a sensible way.


signature.asc
Description: PGP signature

Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.

2021-04-22 Thread Luciana Lima Brito

On Thu, 22 Apr 2021 21:08:08 +0100
Christopher Baines  wrote:

> I'm not quite sure what you mean by empty fields in the JSON here?

I mean now it appears like this in the json

hash-alg:   {}

Before, it was entirely omitted.

> It sounds like you're roughly on the right track, do share what
> changes you're making and then I can have a look.

This is the part I've changed on comparison.scm in the function
derivation-outputs-differences-data:

 (map (match-lambda
 ((output-name path hash-algorithm hash recursive
   derivation_ids)
  (let ((parsed-derivation-ids
 (map string->number
  (parse-postgresql-array-string derivation_ids
(list output-name
  path
  (if (string? hash-algorithm)
  hash-algorithm
  '())
  (if (string? hash)
  hash
  '())
  (string=? recursive "t")
  (append (if (memq base-derivation-id
parsed-derivation-ids)
  '(base)
  '())
  (if (memq target-derivation-id
parsed-derivation-ids)
  '(target)
  '()))
   (exec-query conn query))

Basically, I moved the hash-algorithm, hash, and recursive tests from
controller.scm here. I spent more time familiarizing with this part of
the code and the dataset. I don't have anything ready for the idea I
told you in the last email because I'm not so sure if that is the way
to go, but I can make some experiments in this regard.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.


Luciana Lima Brito  writes:

> Hi,
>
>> Small intentional changes are better, so I'd start just with looking
>> at some of the data coming out of the query. But yes, I think you're
>> in the right place. The hard part here is probably to look at how
>> those values are used in the JSON and HTML rendering code, and adjust
>> that accordingly.
>
> I made some modifications(moved the tests from controller.scm to
> comparison.scm) in order to remove the value "#f" from the outputs
> "hash-algorithm" and "hash" when rendering the html, for derivations
> that don't have these values set, but doing that now I'm showing them
> as empty fields on the json (instead of omitting them entirely).

I'm not quite sure what you mean by empty fields in the JSON here?

> This simplified a bit the processing on render-compare/derivations,
> and I think the html output is also better. For inputs, sources, etc,
> there are no processing being done on controller.scm other than the
> map. I can't see now what else could be done in this regard.
>
> An idea I had was to remove the entire map functions from the
> controller.scm to comparison.scm, but this would impact the html, so I
> would also have to modify the html, because the data format for json and
> html are slightly different. I'm not sure if this is what you want.

It sounds like you're roughly on the right track, do share what changes
you're making and then I can have a look.

Chris


signature.asc
Description: PGP signature

Re: Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes)

2021-04-22 Thread Léo Le Bouter

On Thu, 2021-04-22 at 13:40 -0400, Mark H Weaver wrote:
> This commit was digitally signed and pushed to the 'wip-gnome' branch
> by
> Raghav, but it's also "Signed-off-by: Léo Le Bouter", so I'm not sure
> who bears primary responsibility for this one.

It seems you are more focused and spend more time sending accusations
here than collaboratively working to improve GNU Guix. I don't feel
like that's something great to do at all.

Léo

signature.asc
Description: This is a digitally signed message part

Re: [Outreachy] - Guix Data Service - Set a more informative page title


Canan Talayhan  writes:

> I've missed it unintentionally. I've not touched the "@" sign this time. :)
>
>>>  + (define page-header "Comparing")
>  >>  +
>  >>   (layout
>  >>  +  #:title
>  >>  +  (string-append page-header " " (string-take base-commit 8) " and "
>  >>  +  (string-take target-commit 8))
>  >>#:body
>  >>`(,(header)
>  >> (div
>  >>  @@ -107,7 +112,7 @@
>  >>  (@ (class "col-sm-7"))
>  >>  ,@(if invalid-query?
>  >> `((h1 "Compare"))
>  >>  -   `((h1 "Comparing "
>  >>  +   `((h1 ,page-header ," "
>  >>(a (@ (href ,(string-append "/revision/" base-commit)))
>  >>  (samp ,(string-take base-commit 8) "…"))
>  >>" and "
>
> I think I misunderstood that comment.
>>> There's a couple of things here. I'd be tempted not to use a variable
>>> for "Comparing", it's not really the page header, as that's more
>>> complicated, so I think I'd just use the string in both places.
>
> Now, I've fixed it this way. I hope this version is good.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> *(layout   #:title   (string-append "Comparing " (string-take base-commit
> 8) " and "(string-take target-commit 8))   #:body   `(,(header)
>  (div  (@ (class "container"))  (div   (@ (class "row"))
>  (div(@ (class "col-sm-7")),@(if invalid-query?
>   `((h1 "Compare"))  `((h1 "Comparing "(a
> (@ (href ,(string-append "/revision/" base-commit)))
>  (samp ,(string-take base-commit 8) "…"))" and "*
>

I think your email came through a bit garbled, anyway, I think there's
still an issue with the title here.

> On Mon, Apr 19, 2021 at 10:16 PM Christopher Baines 
> wrote:
>
>>
>> Canan Talayhan  writes:
>>
>> > Thanks for your quick response.
>> >
>> >>Why's the @ being removed here?
>> > It interprets like an HTML code when I use the page-header like
>> > `,page-header, so I removed it. According to your comment, I reverted
>> > to the original version.
>> >
>> > " 'GET repository..." which includes package/package-name in the URL
>> > has not the best titles since I couldn't test them because of the
>> > error that I've mentioned.
>> > I'm open to suggestions.
>> >
>> > Could you please re-review the patch that contains all the
>> > modifications you've mentioned in the previous message?
>>
>> I've had another look, see my comments below.
>>
>> > On Sun, Apr 18, 2021 at 8:53 PM Christopher Baines 
>> wrote:
>> >>
>> >>
>> >> Canan Talayhan  writes:
>> >>
>> >> > I've updated the patch that contains all the suggestions. I think the
>> patch
>> >> > is ready to merge.
>> >> >
>> >> > One thing that I would like to ask you about the package and
>> package-name
>> >> > in web/repository/controller.scm.
>> >> >
>> >> > When I test the URL below I'm getting this error. (
>> >> > https://pastebin.ubuntu.com/p/HdKShmKqH7/)
>> >> >
>> >> >- ('GET "repository" repository-id "branch" branch-name "package"
>> >> >package-name) ->
>> >> >http://localhost:8765/repository/1/branch/master/package/emacs
>> >> >
>> >> > What do you think? BTW it's accessible on the official server.
>> >> >
>> >> >-
>> https://data.guix.gnu.org/repository/1/branch/master/package/emacs/
>> >>
>> >> Hmm, this could possibly be due to an issue with the small dump of the
>> >> database.
>> >>
>> >> > Could you please review the patch attached?
>> >> > I'm very excited to make my first FOSS contribution. :)
>> >>
>> >> I've had a look though, and I have some more comments:
>> >>
>> >>   diff --git a/guix-data-service/web/compare/html.scm
>> b/guix-data-service/web/compare/html.scm
>> >>   index 5b5fe0a..170fb12 100644
>> >>   --- a/guix-data-service/web/compare/html.scm
>> >>   +++ b/guix-data-service/web/compare/html.scm
>> >>   @@ -96,7 +96,12 @@
>> >>(unless invalid-query?
>> >>  (query-parameters->string query-parameters)))
>> >>
>> >>   +  (define page-header "Comparing")
>> >>   +
>> >>  (layout
>> >>   +   #:title
>> >>   +   (string-append page-header " " (string-take base-commit 8) " and "
>> >>   +(string-take target-commit 8))
>> >>   #:body
>> >>   `(,(header)
>> >> (div
>> >>   @@ -107,7 +112,7 @@
>> >>(@ (class "col-sm-7"))
>> >>,@(if invalid-query?
>> >>  `((h1 "Compare"))
>> >>   -  `((h1 "Comparing "
>> >>   +  `((h1 ,page-header ," "
>> >>(a (@ (href ,(string-append "/revision/"
>> base-commit)))
>> >>   (samp ,(string-take base-commit 8) "…"))
>> >>" and "
>> >>
>> >> There's a couple of things here. I'd be tempted not to use a variable
>> >> for "Comparing", it's not really the page header, as that's more
>> >> complicated, so I think I'd just use the string in both places.
>> >>
>> >> Second thing, the (if invalid-query? bit when constructing the h1
>> >> element is important. The query parameters being

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread Léo Le Bouter

On Thu, 2021-04-22 at 00:08 -0400, Mark H Weaver wrote:
> Hi Raghav,
> 
> Raghav Gururajan  writes:
> 
> > > Those commits on 'core-updates' were digitally signed by Léo Le
> > > Bouter
> > >  and have the same problems: they remove
> > > security
> > > fixes, and yet the summary lines indicate that only "cosmetic
> > > changes"
> > > were made.
> > 
> > Yeah, the commit title didn't mention the change but the commit
> > message did.
> 
> I'm sorry, but that won't do.  There are at least three things wrong
> with these commits:
> 
> (1) The summary lines were misleading, because they implied that no
> functional changes were made.
> 
> (2) The commit messages were misleading, because they failed to
> mention
> that security holes which had previously been fixed were now
> being
> re-introduced.  That wasn't at all obvious.
> 
> Commits like these, which remove patches that had fixed security
> flaws, are fairly common: someone casually looking over the
> commit
> log might assume that the patches could be safely removed because
> a
> version update was done at the same time, rendering those patches
> obsolete.
> 
> (3) Although your 'glib' commit was immediately followed by a 'glib'
> update, rendering it harmless, your misleading 'cairo' commit
> left
> 'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our
> 'core-updates' and 'wip-gnome' branches.  Those will need to be
> fixed now.
> 
> Léo Le Bouter  is also culpable here, because he
> digitally signed the misleading 'cairo' commit that's on our
> 'core-updates' branch, which re-introduced CVE-2018-19876 and
> CVE-2020-35492.
> 
> --8<---cut here---start->8---
> commit f94cdc86f644984ca83164d40b17e7eed6e22091
> gpg: Signature made Fri 26 Mar 2021 05:13:57 PM EDT
> gpg:using RSA key
> 148BCB8BD80BFB16B1DE0E9145A8B1E86BCD10A6
> gpg: Good signature from "Léo Le Bouter "
> [unknown]
> gpg: WARNING: This key is not certified with a trusted signature!
> gpg:  There is no indication that the signature belongs to
> the owner.
> Primary key fingerprint: 148B CB8B D80B FB16 B1DE  0E91 45A8 B1E8
> 6BCD 10A6
> Author: Raghav Gururajan 
> Date:   Fri Dec 4 00:48:43 2020 -0500
> 
> gnu: cairo: Make some cosmetic changes.
> 
> * gnu/packages/patches/cairo-CVE-2018-19876.patch,
> gnu/packages/patches/cairo-CVE-2020-35492.patch: Remove patches.
> * gnu/local.mk (dist_patch_DATA): Unregister them.
> * gnu/packages/gtk.scm (cairo): Make some cosmetic changes.
> [replacement]: Remove.
> (cairo/fixed): Remove.
> 
> Signed-off-by: Léo Le Bouter 
> --8<---cut here---end--->8---
> 
> https://git.sv.gnu.org/cgit/guix.git/commit/?h=core-updates=f94cdc86f644984ca83164d40b17e7eed6e22091
> 
> Even the most superficial skimming of this commit should have
> immediately raised red flags, because the summary line is clearly
> inaccurate.  It shows a lack of careful review, to put it mildly.
> 
>   Mark

Hello Mark,

I don't share your analysis, the security fixes werent stripped because
glib/cairo was also updated to latest version in subsequent commits
which were pushed all at once.

Careful review was done, and that's why I signed-off and GPG-signed the
commits. Nobody was put at risk by these commits and no security fixes
were stripped.

Léo


signature.asc
Description: This is a digitally signed message part

Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.

2021-04-22 Thread Luciana Lima Brito

Hi,

> Small intentional changes are better, so I'd start just with looking
> at some of the data coming out of the query. But yes, I think you're
> in the right place. The hard part here is probably to look at how
> those values are used in the JSON and HTML rendering code, and adjust
> that accordingly.

I made some modifications(moved the tests from controller.scm to
comparison.scm) in order to remove the value "#f" from the outputs
"hash-algorithm" and "hash" when rendering the html, for derivations
that don't have these values set, but doing that now I'm showing them
as empty fields on the json (instead of omitting them entirely).
This simplified a bit the processing on render-compare/derivations,
and I think the html output is also better. For inputs, sources, etc,
there are no processing being done on controller.scm other than the
map. I can't see now what else could be done in this regard.

An idea I had was to remove the entire map functions from the
controller.scm to comparison.scm, but this would impact the html, so I
would also have to modify the html, because the data format for json and
html are slightly different. I'm not sure if this is what you want.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

Re: A "cosmetic changes" commit that removes security fixes

Hi Leo,

Leo Famulari  writes:

> On Thu, Apr 22, 2021 at 12:05:36AM -0400, Raghav Gururajan wrote:
>> Okay, I was able to retrace. When Leo and I were working outside savannah,
>> there was master --> core-updates merge. Leo made these changes when he
>> committed to his repo
>> (https://logs.guix.gnu.org/guix/2021-03-26.log#000811), from which I pulled
>> then format-patched and sent it to guix-patches
>> (https://issues.guix.gnu.org/42958#64). From guix-patches it was then pushed
>> to core-updates (https://issues.guix.gnu.org/42958#67), from where I
>> cherry-picked into wip-gnome.
>
> Mark,
>
> Do you know if the security fixes under discussion are necessary on
> core-updates?

The 'cairo' fixes are certainly still needed, because there has been no
upstream stable release of 'cairo' since the version (1.16.0) on our
'master' branch.

宋文武 proposed a patch to re-apply the fixes on 'core-updates', here:

  https://lists.gnu.org/archive/html/guix-devel/2021-04/msg00361.html

A similar patch will be needed for 'wip-gnome' as well.

I'm not sure about the other packages off-hand, but both 'glib' and
'gdk-pixbuf' were ultimately updated to newer versions, so I guess it's
likely that they're okay (although I haven't verified this).

  Thanks,
Mark

-- 
Support Richard Stallman against the vicious misinformation campaign
against him and the FSF.  See  for more.

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread Leo Famulari

On Thu, Apr 22, 2021 at 12:05:36AM -0400, Raghav Gururajan wrote:
> Okay, I was able to retrace. When Leo and I were working outside savannah,
> there was master --> core-updates merge. Leo made these changes when he
> committed to his repo
> (https://logs.guix.gnu.org/guix/2021-03-26.log#000811), from which I pulled
> then format-patched and sent it to guix-patches
> (https://issues.guix.gnu.org/42958#64). From guix-patches it was then pushed
> to core-updates (https://issues.guix.gnu.org/42958#67), from where I
> cherry-picked into wip-gnome.

Mark,

Do you know if the security fixes under discussion are necessary on
core-updates?

Raghav and Léo, is wip-gnome based on core-updates?

Re: Why is glib still grafted on the 'wip-ungrafting' branch? (was Re: wip-ungrafting builds stuck)

2021-04-22 Thread Leo Famulari

On Thu, Apr 22, 2021 at 12:27:52PM -0400, Mark H Weaver wrote:
> I don't understand why it's relevant how many patches are involved.  It
> sounds like if I had concatenated all of the CVE-2021-27219 patches into
> a single file, you would have judged that as "simple", and therefore
> ungrafted it, although it makes no substantive difference.

I know you understand the subtle risks of grafting, compared to
rebuilding packages with the grafted changes. Just because something
works as a graft, or seems to work as a graft, there is no guarantee
that it will continue to work when we absorb the graft and rebuild all
dependent packages.

I decided to use this "simple change" heuristic based on my own
experience working with grafts. Experience grants intuition, and my
intuition tells that me that grafts with fewer lines of changed code are
less likely to cause build failures or to change the behaviour of a
package beyond the desired security fix.

Remember, the goal of this branch was to attempt to *quickly* absorb
some grafts. I had to use a heuristic approach. Both in deciding which
grafts to absorb, and in explaining my decisions to you (I did not
expect you to misunderstand).

I could have told you that I selected these grafts based on "number of
lines of changed code", but it was easier to write "number of patches".

If you had concatenated those patches, I would have noticed that the
file was gigantic and chosen not to ungraft it at this time.

And to preempt the reply that you are sure to send, yes, I actually
looked at the content of the patches when making my decisions.

Another misleading commit log (was Re: A "cosmetic changes" commit that removes security fixes)

Here's another commit with a blatantly misleading commit log:

  
https://git.savannah.gnu.org/cgit/guix.git/commit/?h=wip-gnome=f5fc3c609e2f38ca1c0523deadb9f77d838fbf32

The summary line is "gnu: gdk-pixbuf: Add missing arguments", but in
fact it does all of the following:

(1) Ungrafts 'gdk-pixbuf'.
(2) Updates 'gdk-pixbuf' from 2.40.0 to 2.42.2.
(3) Removes the 'disable-failing-tests' phase.
(4) Adds "#:meson ,meson-0.55" and "#:glib-or-gtk? #t" to the arguments.

Most of the changes above are not mentioned in the commit log at all,
and of course the summary line is extremely misleading.

This commit was digitally signed and pushed to the 'wip-gnome' branch by
Raghav, but it's also "Signed-off-by: Léo Le Bouter", so I'm not sure
who bears primary responsibility for this one.

  Mark

-- 
Support Richard Stallman against the vicious misinformation campaign
against him and the FSF.  See  for more.

Re: A "cosmetic changes" commit that removes security fixes

Hi Raghav,

Raghav Gururajan  writes:

> Okay, I was able to retrace. When Leo and I were working outside 
> savannah, there was master --> core-updates merge. Leo made these 
> changes when he committed to his repo 
> (https://logs.guix.gnu.org/guix/2021-03-26.log#000811), from which I 
> pulled then format-patched and sent it to guix-patches 
> (https://issues.guix.gnu.org/42958#64). From guix-patches it was then 
> pushed to core-updates (https://issues.guix.gnu.org/42958#67), from 
> where I cherry-picked into wip-gnome.

Thank you for these links.  From the IRC log cited above, it now appears
that Léo Le Bouter  bears primary responsibility
for these mistakes.  In particular, according to the IRC
logs, Léo wrote:

   raghavgururajan: the main issues on the rebasing were about
  security fixes on cairo, gdk-pixbuf and glib
   I modified the cosmetic commits to remove the graft and
  patches etc.

> It seems Leo made these for ungrafting. I not familiar with ungrafting, 
> so I have to let Leo explain.

Yes, I would very much like to hear an explanation from Léo about how
this happened.

Nonetheless, you (Raghav) also bear some responsibility for digitally
signing and pushing these misleading commits to the 'wip-gnome' branch.
At least one of the problems (the misleading summary line) should have
been obvious from a cursory glance at the commit log.

  Mark

--
Support Richard Stallman against the vicious misinformation campaign
against him and the FSF.  See  for more.

Re: [bug#47615] [PATCH v2 09/12] gnu: mercurial: Skip tests on powerpc-linux.

2021-04-22 Thread Maxime Devos

Efraim Flashner schreef op do 22-04-2021 om 10:59 [+0300]:
> * gnu/packages/version-control.scm (mercurial)[arguments]: Skip tests on
> powerpc-linux.
> ---
>  gnu/packages/version-control.scm | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Unchanged since last patchset, IMO not ready for upstreaming.
> 
> diff --git a/gnu/packages/version-control.scm 
> b/gnu/packages/version-control.scm
> index 4d4b276a10..13e2ccd400 100644
> --- a/gnu/packages/version-control.scm
> +++ b/gnu/packages/version-control.scm
> @@ -1688,7 +1688,11 @@ execution of any hook written in any language before 
> every commit.")
>   "--slowtimeout" "86400"
>   ;; The test suite takes a long time and produces 
> little
>   ;; output by default.  Prevent timeouts due to 
> silence.
> - "-v"
> + "-v"))
> +   ;; Tests on powerpc-linux take more than 10 hours.
> +   #:tests? ,(if (string=? "powerpc-linux" (or (%current-system)
> +   (%current-target-system)))
> +   '#f '#t)))

You can remove ' in '#f and '#t here.
That shouldn't cause a derivation change.
Keeping ' is harmless though, it's just superfluous.
Likewise for some other commits.

I'm not really familiar with powerpc and the packages you modified,
so I'm just looking at style, and cross-compilation issues (which you seem
to have covered).

Greetings,
Maxime.


signature.asc
Description: This is a digitally signed message part

Re: Why is glib still grafted on the 'wip-ungrafting' branch? (was Re: wip-ungrafting builds stuck)

Hi Leo,

Leo Famulari  writes:

> On Wed, Apr 21, 2021 at 04:47:06PM -0400, Mark H Weaver wrote:
>> I just noticed that 'glib' is still grafted on the 'wip-ungrafting'
>> branch.  Was that intentional?
>> 
>> https://git.sv.gnu.org/cgit/guix.git/tree/gnu/packages/glib.scm?h=wip-ungrafting=e12210dc92098d8581cea3007d57dbb6be16bb41#n171
>
> Yes. For that branch I only selected grafts that I judged to be
> "simple". There are many other grafts still in place on that branch.

Okay, thanks for the explanation.

> My criteria for simplicity are grafts that either apply one or two
> patches, or are minor version upgrades of projects that are known to
> care about ABI compatibility.

I don't understand why it's relevant how many patches are involved.  It
sounds like if I had concatenated all of the CVE-2021-27219 patches into
a single file, you would have judged that as "simple", and therefore
ungrafted it, although it makes no substantive difference.

Anyway, it makes no difference to me; I'll continue doing my own thing
on my private branch.  I just wanted to make sure that it wasn't an
oversight.

 Thanks,
   Mark

licensing problem with "On Lisp" and code from a github gist

2021-04-22 Thread Giovanni Biscuolo

Hi,

we have a couple of issues with a new proposed interesting package:
Weir, written in Common Lisp.

This is the proposed patch:
https://issues.guix.gnu.org/issue/47943

There are a couple of problems:

1. the code from the book "On Lisp" by Paul Graham is not free software
(AFAIU): please any Common Lisp programmes can tell us (ehrm, upstream
actually) if the "On Lisp" code has been "superseded" by some proper cl
library?

2. the code from this gist is included upstream:
The code referenced in 3. is suggested by lispm in
https://gist.github.com/inconvergent/8b6ccfbde4fca7844c1962082ef07a7e
and AFAIU is not a trivial patch and the license is not stated, so
should be "Full Copyright".

Please see the issue tracker for details.

WDYT?

Thanks, Giovanni.

-- 
Giovanni Biscuolo

Xelera IT Infrastructures


signature.asc
Description: PGP signature

Re: A "cosmetic changes" commit that removes security fixes

Hi,

宋文武  writes:

> This patch is for core-updates:
> From 15e28e84eaea8f68b6247ab53052f0dd50a544b2 Mon Sep 17 00:00:00 2001
> From: 宋文武 
> Date: Thu, 22 Apr 2021 19:21:51 +0800
> Subject: [PATCH] gnu: cairo: Reintroduce security patches [security fixes].
>
> Two patches were accidentally removed in commit
> f94cdc86f644984ca83164d40b17e7eed6e22091.
>
> * gnu/packages/patches/cairo-CVE-2018-19876.patch,
> gnu/packages/patches/cairo-CVE-2020-35492.patch: New files.
> * gnu/local.mk (dist_patch_DATA): Add them.
> * gnu/packages/gtk.scm (cairo)[patches]: Apply them.

Looks good to me.  Please push!

Thanks,
  Mark

Re: A "cosmetic changes" commit that removes security fixes

2021-04-22 Thread 宋文武

Mark H Weaver  writes:

> Hi Raghav,
>
> Raghav Gururajan  writes:
>
>>> Those commits on 'core-updates' were digitally signed by Léo Le Bouter
>>>  and have the same problems: they remove security
>>> fixes, and yet the summary lines indicate that only "cosmetic changes"
>>> were made.
>>
>> Yeah, the commit title didn't mention the change but the commit message did.
>
> I'm sorry, but that won't do.  There are at least three things wrong
> with these commits:
>
> (1) The summary lines were misleading, because they implied that no
> functional changes were made.

Yes, if the title can't summary the change, then the change should be
splited into multiple commits.
>
> (2) The commit messages were misleading, because they failed to mention
> that security holes which had previously been fixed were now being
> re-introduced.  That wasn't at all obvious.
>
> Commits like these, which remove patches that had fixed security
> flaws, are fairly common: someone casually looking over the commit
> log might assume that the patches could be safely removed because a
> version update was done at the same time, rendering those patches
> obsolete.

Agree, I think we should mention explicitly that those patches are now
not needed after some code audit.
>
> (3) Although your 'glib' commit was immediately followed by a 'glib'
> update, rendering it harmless, your misleading 'cairo' commit left
> 'cairo' vulnerable to CVE-2018-19876 and CVE-2020-35492 on our
> 'core-updates' and 'wip-gnome' branches.  Those will need to be
> fixed now.

This patch is for core-updates:
>From 15e28e84eaea8f68b6247ab53052f0dd50a544b2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=AE=8B=E6=96=87=E6=AD=A6?= 
Date: Thu, 22 Apr 2021 19:21:51 +0800
Subject: [PATCH] gnu: cairo: Reintroduce security patches [security fixes].

Two patches were accidentally removed in commit
f94cdc86f644984ca83164d40b17e7eed6e22091.

* gnu/packages/patches/cairo-CVE-2018-19876.patch,
gnu/packages/patches/cairo-CVE-2020-35492.patch: New files.
* gnu/local.mk (dist_patch_DATA): Add them.
* gnu/packages/gtk.scm (cairo)[patches]: Apply them.
---
 gnu/local.mk  |  2 +
 gnu/packages/gtk.scm  |  5 +-
 .../patches/cairo-CVE-2018-19876.patch| 37 ++
 .../patches/cairo-CVE-2020-35492.patch| 49 +++
 4 files changed, 92 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/cairo-CVE-2018-19876.patch
 create mode 100644 gnu/packages/patches/cairo-CVE-2020-35492.patch

diff --git a/gnu/local.mk b/gnu/local.mk
index a8dd66f34a..39b2b72a42 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -880,6 +880,8 @@ dist_patch_DATA =		\
   %D%/packages/patches/bpftrace-disable-bfd-disasm.patch	\
   %D%/packages/patches/busybox-CVE-2021-28831.patch		\
   %D%/packages/patches/byobu-writable-status.patch		\
+  %D%/packages/patches/cairo-CVE-2018-19876.patch		\
+  %D%/packages/patches/cairo-CVE-2020-35492.patch		\
   %D%/packages/patches/calibre-no-updates-dialog.patch		\
   %D%/packages/patches/calibre-remove-test-sqlite.patch		\
   %D%/packages/patches/calibre-remove-test-unrar.patch		\
diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index 9f3aea4aca..f70e667115 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -142,7 +142,10 @@ tools have full access to view and control running applications.")
 (string-append "https://cairographics.org/releases/cairo-;
version ".tar.xz"))
(sha256
-(base32 "0c930mk5xr2bshbdljv005j3j8zr47gqmkry3q6qgvqky6rjjysy"
+(base32 "0c930mk5xr2bshbdljv005j3j8zr47gqmkry3q6qgvqky6rjjysy"))
+   (patches (search-patches
+		 "cairo-CVE-2018-19876.patch"
+		 "cairo-CVE-2020-35492.patch"
 (build-system glib-or-gtk-build-system)
 (outputs '("out" "doc"))
 (arguments
diff --git a/gnu/packages/patches/cairo-CVE-2018-19876.patch b/gnu/packages/patches/cairo-CVE-2018-19876.patch
new file mode 100644
index 00..c0fba2ecaa
--- /dev/null
+++ b/gnu/packages/patches/cairo-CVE-2018-19876.patch
@@ -0,0 +1,37 @@
+Copied from Debian.
+
+From: Carlos Garcia Campos 
+Date: Mon, 19 Nov 2018 12:33:07 +0100
+Subject: ft: Use FT_Done_MM_Var instead of free when available in
+ cairo_ft_apply_variations
+
+Fixes a crash when using freetype >= 2.9
+
+[This is considered to be security-sensitive because WebKitGTK+ sets its
+own memory allocator, which is not compatible with system free(), making
+this a remotely triggerable denial of service or memory corruption.]
+
+Origin: upstream, commit:90e85c2493fdfa3551f202ff10282463f1e36645
+Bug: https://gitlab.freedesktop.org/cairo/cairo/merge_requests/5
+Bug-Debian: https://bugs.debian.org/916389
+Bug-CVE: CVE-2018-19876
+---
+ src/cairo-ft-font.c | 4 
+ 1 file changed, 4 insertions(+)
+
+diff --git a/src/cairo-ft-font.c b/src/cairo-ft-font.c
+index 325dd61..981973f 100644
+---

[PATCH v2 12/12] gnu: glib: Disable failing test.

* gnu/packages/glib.scm (glib)[source]: Add patch.
[arguments]: Remove custom 'increase-test-timeout phase.
* gnu/packages/patches/glib-skip-failing-test.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
---
 gnu/local.mk  |  1 +
 gnu/packages/glib.scm | 16 +++
 .../patches/glib-skip-failing-test.patch  | 27 +++
 3 files changed, 31 insertions(+), 13 deletions(-)
 create mode 100644 gnu/packages/patches/glib-skip-failing-test.patch

I don't have a way to test this on native armhf or i686 but I tried
changing the test timeout to 30 minutes (from the new default of 180
seconds) and it still timed out / hung forever. This is the minimal
patch to skip that test.

diff --git a/gnu/local.mk b/gnu/local.mk
index 7aba045e4b..18fa325503 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1080,6 +1080,7 @@ dist_patch_DATA = 
\
   %D%/packages/patches/ghostscript-no-header-uuid.patch\
   %D%/packages/patches/ghostscript-no-header-creationdate.patch \
   %D%/packages/patches/glib-appinfo-watch.patch\
+  %D%/packages/patches/glib-skip-failing-test.patch\
   %D%/packages/patches/glibc-CVE-2018-11236.patch  \
   %D%/packages/patches/glibc-CVE-2018-11237.patch  \
   %D%/packages/patches/glibc-CVE-2019-7309.patch   \
diff --git a/gnu/packages/glib.scm b/gnu/packages/glib.scm
index 34d2ba4cf4..ede4a06a08 100644
--- a/gnu/packages/glib.scm
+++ b/gnu/packages/glib.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2013, 2015 Andreas Enge 
 ;;; Copyright © 2013 Nikita Karetnikov 
 ;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2021 Mark H Weaver 

-;;; Copyright © 2016, 2020 Efraim Flashner 
+;;; Copyright © 2016, 2020, 2021 Efraim Flashner 
 ;;; Copyright © 2016 Lukas Gradl 
 ;;; Copyright © 2017, 2018, 2019 Ricardo Wurmus 
 ;;; Copyright © 2017 Petter 
@@ -187,7 +187,8 @@ shared NFS home directories.")
(sha256
 (base32 "1sh3h6b734cxhdd1qlzvhxq6rc7k73dsisap5y3s419s9xc4ywv7"))
(patches
-(search-patches "glib-appinfo-watch.patch"))
+(search-patches "glib-appinfo-watch.patch"
+"glib-skip-failing-test.patch"))
(modules '((guix build utils)))
(snippet
 '(begin
@@ -203,17 +204,6 @@ shared NFS home directories.")
#:configure-flags '("-Dman=true")
#:phases
(modify-phases %standard-phases
- ;; TODO: Remove the conditional in the next core-updates cycle.
- ;; Needed to build glib on slower ARM nodes.
- ,@(if (string-prefix? "arm" (%current-system))
-   `((add-after 'unpack 'increase-test-timeout
-   (lambda _
- (substitute* "meson.build"
-   (("test_timeout = 60")
-"test_timeout = 90")
-   (("test_timeout_slow = 120")
-"test_timeout_slow = 180")
-   '())
  (add-after 'unpack 'disable-failing-tests
(lambda _
  (with-directory-excursion "glib/tests"
diff --git a/gnu/packages/patches/glib-skip-failing-test.patch 
b/gnu/packages/patches/glib-skip-failing-test.patch
new file mode 100644
index 00..c7706aaa74
--- /dev/null
+++ b/gnu/packages/patches/glib-skip-failing-test.patch
@@ -0,0 +1,27 @@
+This test timed out on powerpc-linux even after extending the
+test_timeout_slow to 1800 seconds. Previously we tried to work around
+this test by extending test_timeout_slow by 1.5 its previous value.
+
+---
+ gio/tests/meson.build | 4 
+ 1 file changed, 4 deletions(-)
+
+diff --git a/gio/tests/meson.build b/gio/tests/meson.build
+index a926ae0..4fdbe7a 100644
+--- a/gio/tests/meson.build
 b/gio/tests/meson.build
+@@ -317,10 +317,6 @@ if host_machine.system() != 'windows'
+ 'extra_sources' : [extra_sources, gdbus_test_codegen_generated, 
gdbus_test_codegen_generated_interface_info],
+ 'c_args' : ['-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_32'],
+   },
+-  'gdbus-threading' : {
+-'extra_sources' : extra_sources,
+-'suite' : ['slow'],
+-  },
+   'gmenumodel' : {
+ 'extra_sources' : extra_sources,
+ 'suite' : ['slow'],
+
+-- 
+2.31.1
+
-- 
2.31.1

[PATCH v2 11/12] gnu: zstd: Adjust test suite for 32-bit architectures.

* gnu/packages/compression.scm (zstd)[arguments]: Adjust
'fix-tests-32bit phase to work better on low RAM machines.
---
 gnu/packages/compression.scm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Discussed on IRC, ready to go straight to core-updates.

diff --git a/gnu/packages/compression.scm b/gnu/packages/compression.scm
index e28b78a3fa..1941ddd43f 100644
--- a/gnu/packages/compression.scm
+++ b/gnu/packages/compression.scm
@@ -7,7 +7,7 @@
 ;;; Copyright © 2015, 2016, 2017, 2018, 2020 Ricardo Wurmus 

 ;;; Copyright © 2015, 2017, 2018 Leo Famulari 
 ;;; Copyright © 2015 Jeff Mickey 
-;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020 Efraim Flashner 

+;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020, 2021 Efraim Flashner 

 ;;; Copyright © 2016 Ben Woodcroft 
 ;;; Copyright © 2016 Danny Milosavljevic 
 ;;; Copyright © 2016–2021 Tobias Geerinckx-Rice 
@@ -1422,7 +1422,7 @@ or junctions, and always follows hard links.")
(lambda _
  (substitute* "tests/playTests.sh"
(("roundTripTest -g8M \"19 -T0 --long\"")
-"roundTripTest -g8M \"22 -T0 --long\""
+"roundTripTest -g8M \"16 -T0 --long\""
  (add-after 'unpack 'remove-bogus-check
(lambda _
  ;; lib/Makefile falsely claims that no .pc file can be created.
-- 
2.31.1

[PATCH v2 10/12] gnu: nss: Skip tests on powerpc-linux.

* gnu/packages/nss.scm (nss)[arguments]: Skip tests on powerpc-linux.
---
 gnu/packages/nss.scm | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

Unchanged since last patchset, IMO not ready for upstreaming.

diff --git a/gnu/packages/nss.scm b/gnu/packages/nss.scm
index e054363e9f..954a1d3b80 100644
--- a/gnu/packages/nss.scm
+++ b/gnu/packages/nss.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019 Ludovic Courtès 

 ;;; Copyright © 2014, 2015, 2016, 2017, 2018, 2019 Mark H Weaver 

-;;; Copyright © 2016, 2017, 2018, 2019 Efraim Flashner 
+;;; Copyright © 2016, 2017, 2018, 2019, 2020 Efraim Flashner 

 ;;; Copyright © 2017, 2018 Tobias Geerinckx-Rice 
 ;;; Copyright © 2020 Marius Bakke 
 ;;; Copyright © 2020 Jonathan Brielmaier 
@@ -109,7 +109,10 @@ in the Mozilla clients.")
;; Parallel builds are not supported (see:
;; https://bugzilla.mozilla.org/show_bug.cgi?id=1640328).
`(#:parallel-build? #f
- #:tests? #t   ;requires at least one hour to run
+ ;; Tests on powerpc-linux take forever and fail sporatically.
+ #:tests? ,(if (string=? "powerpc-linux" (or (%current-system)
+ (%current-target-system)))
+ '#f '#t)
  #:make-flags
  (let* ((out (assoc-ref %outputs "out"))
 (nspr (string-append (assoc-ref %build-inputs "nspr")))
-- 
2.31.1

[PATCH v2 09/12] gnu: mercurial: Skip tests on powerpc-linux.

* gnu/packages/version-control.scm (mercurial)[arguments]: Skip tests on
powerpc-linux.
---
 gnu/packages/version-control.scm | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

Unchanged since last patchset, IMO not ready for upstreaming.

diff --git a/gnu/packages/version-control.scm b/gnu/packages/version-control.scm
index 4d4b276a10..13e2ccd400 100644
--- a/gnu/packages/version-control.scm
+++ b/gnu/packages/version-control.scm
@@ -1688,7 +1688,11 @@ execution of any hook written in any language before 
every commit.")
  "--slowtimeout" "86400"
  ;; The test suite takes a long time and produces 
little
  ;; output by default.  Prevent timeouts due to 
silence.
- "-v"
+ "-v"))
+   ;; Tests on powerpc-linux take more than 10 hours.
+   #:tests? ,(if (string=? "powerpc-linux" (or (%current-system)
+   (%current-target-system)))
+   '#f '#t)))
 ;; The following inputs are only needed to run the tests.
 (native-inputs
  `(("python-nose" ,python-nose)
-- 
2.31.1

[PATCH v2 08/12] build: qemu-command: Add support for powerpc.

* gnu/build/vm.scm (qemu-command): Add missing case for powerpc.
---
 gnu/build/vm.scm | 1 +
 1 file changed, 1 insertion(+)

Adjusted at the suggestion of Vincent Legoll to strongly match just
powerpc-linux and not powerpc64le.

diff --git a/gnu/build/vm.scm b/gnu/build/vm.scm
index 253d9bcd31..88b27adb4f 100644
--- a/gnu/build/vm.scm
+++ b/gnu/build/vm.scm
@@ -75,6 +75,7 @@
(cond
 ((string-match "^i[3456]86$" cpu) "i386")
 ((string-match "armhf" cpu) "arm")
+((string-match "^powerpc$" cpu) "ppc")
 (else cpu)
 
 (define* (load-in-linux-vm builder
-- 
2.31.1

[PATCH v2 07/12] gnu: american-fuzzy-lop: Add support for powerpc-linux.

* gnu/packages/debug.scm (american-fuzzy-lop): Add case for
powerpc-linux.
(qemu-for-american-fuzzy-lop): Same.
---
 gnu/packages/debug.scm | 2 ++
 1 file changed, 2 insertions(+)

This patch is already in master and already merged into core-updates.

diff --git a/gnu/packages/debug.scm b/gnu/packages/debug.scm
index 2913c348f3..1326ce6e16 100644
--- a/gnu/packages/debug.scm
+++ b/gnu/packages/debug.scm
@@ -179,6 +179,7 @@ tools that process C/C++ code.")
("aarch64-linux"  "aarch64")
("armhf-linux""arm")
("mips64el-linux" "mips64el")
+   ("powerpc-linux"  "ppc")
;; Prevent errors when querying this package on unsupported
;; platforms, e.g. when running "guix package --search="
(_"UNSUPPORTED"
@@ -254,6 +255,7 @@ down the road.")
("aarch64-linux"  "aarch64")
("armhf-linux""arm")
("mips64el-linux" "mips64el")
+   ("powerpc-linux"  "ppc")
;; Prevent errors when querying this package on unsupported
;; platforms, e.g. when running "guix package --search="
(_"UNSUPPORTED"
-- 
2.31.1

[PATCH v2 06/12] gnu: Add mac-fdisk.

* gnu/packages/disk.scm (mac-fdisk): New variable.
* gnu/packages/patches/mac-fdisk-gentoo-patchset.patch,
gnu/packages/patches/mac-fdisk-p18.patch: New files.
* gnu/local.mk (dist_patch_DATA): Register them.
---
 gnu/local.mk  |2 +
 gnu/packages/disk.scm |   43 +
 .../patches/mac-fdisk-gentoo-patchset.patch   |  866 +++
 gnu/packages/patches/mac-fdisk-p18.patch  | 2070 +
 4 files changed, 2981 insertions(+)
 create mode 100644 gnu/packages/patches/mac-fdisk-gentoo-patchset.patch
 create mode 100644 gnu/packages/patches/mac-fdisk-p18.patch

I'm fairly certain there's just the cc-for-target change to make-flags.
This package is still untested on actual hardware.

diff --git a/gnu/local.mk b/gnu/local.mk
index 23b768871c..7aba045e4b 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1351,6 +1351,8 @@ dist_patch_DATA = 
\
   %D%/packages/patches/luit-posix.patch\
   %D%/packages/patches/lvm2-static-link.patch  \
   %D%/packages/patches/mailutils-fix-uninitialized-variable.patch  \
+  %D%/packages/patches/mac-fdisk-gentoo-patchset.patch \
+  %D%/packages/patches/mac-fdisk-p18.patch \
   %D%/packages/patches/make-impure-dirs.patch  \
   %D%/packages/patches/mars-install.patch  \
   %D%/packages/patches/mars-sfml-2.3.patch \
diff --git a/gnu/packages/disk.scm b/gnu/packages/disk.scm
index ed112f2ec2..5f24d555f0 100644
--- a/gnu/packages/disk.scm
+++ b/gnu/packages/disk.scm
@@ -302,6 +302,49 @@ fdisk.  fdisk is used for the creation and manipulation of 
disk partition
 tables, and it understands a variety of different formats.")
 (license license:gpl3+)))
 
+(define-public mac-fdisk
+  (package
+(name "mac-fdisk")
+(version "0.1")
+(source
+  (origin
+(method url-fetch)
+(uri (string-append "mirror://debian/pool/main/m/mac-fdisk/"
+"mac-fdisk_" version ".orig.tar.gz"))
+(sha256
+ (base32 "0rkaqp82l47pg0ymqys07mljf3widv2yk4hhgs2yz8hwli5zqnbh"))
+(patches (search-patches "mac-fdisk-p18.patch"
+ "mac-fdisk-gentoo-patchset.patch"
+(build-system gnu-build-system)
+(arguments
+ `(#:phases
+   (modify-phases %standard-phases
+ (delete 'configure); no configure script.
+ (replace 'install
+   (lambda* (#:key outputs #:allow-other-keys)
+ (let* ((out  (assoc-ref outputs "out"))
+(sbin (string-append out "/sbin"))
+(man8 (string-append out "/share/man/man8")))
+   (mkdir-p sbin)
+   (mkdir-p man8)
+   (copy-file "fdisk" (string-append sbin "/mac-fdisk"))
+   (copy-file "pdisk" (string-append sbin "/pmac-fdisk"))
+   (copy-file "mac-fdisk.8.in" (string-append man8 "/mac-fdisk.8"))
+   (copy-file "pmac-fdisk.8.in" (string-append man8 
"/pmac-fdisk.8"))
+   #:make-flags (list (string-append "CC=" ,(cc-for-target)))
+   #:tests? #f)); no tests
+(home-page "https://tracker.debian.org/pkg/mac-fdisk;)
+(synopsis "Apple disk partition manipulation tool")
+(description "The @code{fdisk} utilities from the MkLinux project, adopted
+for Linux/m68k.  @code{mac-fdisk} allows you to create and edit the partition
+table of a disk.  It supports only the Apple partition format used on Macintosh
+and PowerMac, use @code{pmac-fdisk} for PC partition format disks as used on
+PowerPC machines.  @code{mac-fdisk} is an interactive tool with a menu similar
+to PC @code{fdisk}, supported options are somewhat different from PC
+@code{fdisk} due to the differences in partition format.")
+(supported-systems '("powerpc-linux" "i686-linux" "x86_64-linux"))
+(license license:gpl2)))
+
 (define-public gptfdisk
   (package
 (name "gptfdisk")
diff --git a/gnu/packages/patches/mac-fdisk-gentoo-patchset.patch 
b/gnu/packages/patches/mac-fdisk-gentoo-patchset.patch
new file mode 100644
index 00..b1bd38f671
--- /dev/null
+++ b/gnu/packages/patches/mac-fdisk-gentoo-patchset.patch
@@ -0,0 +1,866 @@
+https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-fs/mac-fdisk/files
+
+---
+ bitfield.c  | 16 +-
+ bitfield.h  |  4 +--
+ dump.c  | 42 --
+ errors.c| 11 ---
+ fdisk.c | 68 ++
+ fdisk.h |  5 
+ fdisklabel.c| 79 +++--
+ fdisklabel.h|  2 +-
+ io.c|  8 +++--
+ kernel-defs.h   |  6 
+ partition_map.c | 43 ---
+ pdisk.c |  7 ++---
+ 12 files changed, 186 insertions(+), 105 deletions(-)
+
+diff --git a/bitfield.c b/bitfield.c
+index 687e8bd..02e7f68

[PATCH v2 05/12] gnu: mesa: Add support for powerpc-linux.

* gnu/packages/gl.scm (mesa)[inputs]: Add llvm, glslang for powerpc.
[arguments]: Customize the configure flags for powerpc. Add powerpc
specific phase to skip failing tests.
---
 gnu/packages/gl.scm | 33 +
 1 file changed, 29 insertions(+), 4 deletions(-)

Blanket skipping of the test suite removed, instead we have a phase to
skip certain tests. When rebased on core-updates there will be 3 such
phases and adjustments to the packaging already for powerpc64le and
aarch64. I'll make adjustments to that when I rebase next.

diff --git a/gnu/packages/gl.scm b/gnu/packages/gl.scm
index c194d269f8..ec76029cf9 100644
--- a/gnu/packages/gl.scm
+++ b/gnu/packages/gl.scm
@@ -271,7 +271,7 @@ also known as DXTn or DXTC) for Mesa.")
 ("libxrandr" ,libxrandr)
 ("libxvmc" ,libxvmc)
 ,@(match (%current-system)
-((or "x86_64-linux" "i686-linux")
+((or "x86_64-linux" "i686-linux" "powerpc-linux")
  ;; Note: update the 'clang' input of mesa-opencl when bumping 
this.
  `(("llvm" ,llvm-11)))
 (_
@@ -283,7 +283,7 @@ also known as DXTn or DXTC) for Mesa.")
 ("flex" ,flex)
 ("gettext" ,gettext-minimal)
 ,@(match (%current-system)
-((or "x86_64-linux" "i686-linux")
+((or "x86_64-linux" "i686-linux" "powerpc-linux")
  `(("glslang" ,glslang)))
 (_
  `()))
@@ -298,6 +298,8 @@ also known as DXTn or DXTC) for Mesa.")
  ((or "armhf-linux" "aarch64-linux")
   ;; TODO: Fix svga driver for aarch64 and armhf.
   
'("-Dgallium-drivers=etnaviv,freedreno,kmsro,lima,nouveau,panfrost,r300,r600,swrast,tegra,v3d,vc4,virgl"))
+ ("powerpc-linux"
+  '("-Dgallium-drivers=nouveau,r300,r600,radeonsi,swrast,virgl"))
  (_
   
'("-Dgallium-drivers=iris,nouveau,r300,r600,radeonsi,svga,swrast,virgl")))
  ;; Enable various optional features.  TODO: opencl requires libclc,
@@ -318,12 +320,14 @@ also known as DXTn or DXTC) for Mesa.")
  ,@(match (%current-system)
  ((or "i686-linux" "x86_64-linux")
   '("-Dvulkan-drivers=intel,amd"))
+ ("powerpc-linux"   ; No default on this platform.
+  '("-Dvulkan-drivers=amd"))
  (_
   '("-Dvulkan-drivers=auto")))
 
  ;; Enable the Vulkan overlay layer on i686-linux and x86-64-linux.
  ,@(match (%current-system)
- ((or "x86_64-linux" "i686-linux")
+ ((or "x86_64-linux" "i686-linux" "powerpc-linux")
   '("-Dvulkan-overlay-layer=true"))
  (_
   '()))
@@ -337,6 +341,9 @@ also known as DXTn or DXTC) for Mesa.")
  ((or "x86_64-linux" "i686-linux")
   '("-Ddri-drivers=i915,i965,nouveau,r200,r100"
 "-Dllvm=enabled"))  ; default is x86/x86_64 only
+ ("powerpc-linux"
+  '("-Ddri-drivers=nouveau,r200,r100"
+"-Dllvm=enabled"))
  (_
   '("-Ddri-drivers=nouveau,r200,r100"
 
@@ -350,6 +357,24 @@ also known as DXTn or DXTC) for Mesa.")
   (guix build meson-build-system))
#:phases
(modify-phases %standard-phases
+ ,@(if (string-prefix? "powerpc-" (or (%current-target-system)
+  (%current-system)))
+   ;; There are some tests which fail specifically on powerpc.
+   `((add-after 'unpack 'disable-failing-test
+   (lambda _
+ (substitute* '(;; LLVM ERROR: Relocation type not 
implemented yet!
+"src/gallium/drivers/llvmpipe/meson.build"
+;; This is probably a big-endian test 
failure.
+"src/gallium/targets/osmesa/meson.build")
+   (("if with_tests") "if not with_tests"))
+ (substitute* "src/util/tests/format/meson.build"
+   ;; This is definately an endian-ness test failure.
+   (("'u_format_test', ") ""))
+ ;; It is only this portion of the test which fails.
+ (substitute* "src/mesa/main/tests/meson.build"
+   ((".*mesa_formats.*") ""))
+ #t)))
+   '())
  ,@(if (string-prefix? "i686" (or (%current-target-system)
   (%current-system)))
;; Disable new test from Mesa 19 that fails on i686.  Upstream
@@ -390,7 +415,7 @@ also known as DXTn or DXTC) for Mesa.")
  (let ((out (assoc-ref outputs "out"))
(bin (assoc-ref outputs "bin")))
,@(match (%current-system)
-   ((or "i686-linux" "x86_64-linux")
+   ((or "i686-linux" "x86_64-linux"

[PATCH v2 04/12] gnu: binutils: Fix bug in test suite in libiberty.

* gnu/packages/base.scm (binutils)[source]: Add patch.
* gnu/packages/patches/binutils-libiberty-endianness-bug.patch: New file.
* gnu/local.mk (dist_patch_DATA): Register it.
---
 gnu/local.mk  |  1 +
 gnu/packages/base.scm |  3 +-
 .../binutils-libiberty-endianness-bug.patch   | 36 +++
 3 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 gnu/packages/patches/binutils-libiberty-endianness-bug.patch

Completely rewritten. I reported the bug upstream to binutils and after
some testing it was determined to be an actual bug in libiberty's test
suite affecting big-endian systems. I've tested this on x86_64 and saw
no issues with it.

diff --git a/gnu/local.mk b/gnu/local.mk
index f9996e6fa1..23b768871c 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -865,6 +865,7 @@ dist_patch_DATA =   
\
   %D%/packages/patches/biber-sortinithash.patch\
   %D%/packages/patches/bidiv-update-fribidi.patch  \
   %D%/packages/patches/binutils-boot-2.20.1a.patch \
+  %D%/packages/patches/binutils-libiberty-endianness-bug.patch \
   %D%/packages/patches/binutils-loongson-workaround.patch  \
   %D%/packages/patches/binutils-mingw-w64-timestamp.patch  \
   %D%/packages/patches/binutils-mingw-w64-deterministic.patch  \
diff --git a/gnu/packages/base.scm b/gnu/packages/base.scm
index ee1ab1bcad..3dd4f2d0be 100644
--- a/gnu/packages/base.scm
+++ b/gnu/packages/base.scm
@@ -507,7 +507,8 @@ change.  GNU make offers many powerful extensions over the 
standard utility.")
 (sha256
  (base32
   "18ypqr5y48vxqg9mkz1c47798jp1xb1d4vfpmfq8vkihkvkx4jsv"))
-(patches (search-patches "binutils-loongson-workaround.patch"
+(patches (search-patches "binutils-libiberty-endianness-bug.patch"
+ "binutils-loongson-workaround.patch"
(build-system gnu-build-system)
 
;; TODO: Add dependency on zlib + those for Gold.
diff --git a/gnu/packages/patches/binutils-libiberty-endianness-bug.patch 
b/gnu/packages/patches/binutils-libiberty-endianness-bug.patch
new file mode 100644
index 00..e6c82f704e
--- /dev/null
+++ b/gnu/packages/patches/binutils-libiberty-endianness-bug.patch
@@ -0,0 +1,36 @@
+This patch fixes a bug exposed when running the libiberty test suite on
+big-endian machines.
+
+Original bug report:
+https://sourceware.org/bugzilla/show_bug.cgi?id=27751
+Follow-ups:
+https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100177
+https://gcc.gnu.org/pipermail/gcc-patches/2021-April/568314.html
+
+---
+ libiberty/rust-demangle.c | 9 ++---
+ 1 file changed, 6 insertions(+), 3 deletions(-)
+
+diff --git a/libiberty/rust-demangle.c b/libiberty/rust-demangle.c
+index 6fd8f6a4db0..848563fa3c3 100644
+--- a/libiberty/rust-demangle.c
 b/libiberty/rust-demangle.c
+@@ -1253,9 +1253,12 @@ demangle_const_char (struct rust_demangler *rdm)
+   else if (value == '\n')
+ PRINT ("\\n");
+   else if (value > ' ' && value < '~')
+-/* Rust also considers many non-ASCII codepoints to be printable, but
+-   that logic is not easily ported to C. */
+-print_str (rdm, (char *) , 1);
++{
++  /* Rust also considers many non-ASCII codepoints to be printable, but
++  that logic is not easily ported to C. */
++  char c = value;
++  print_str (rdm, , 1);
++}
+   else
+ {
+   PRINT ("\\u{");
+-- 
+2.31.1
+
-- 
2.31.1

[PATCH v2 03/12] gnu: gcc-boot0: Use 128-bit long-double on powerpc-linux.

* gnu/packages/commencement.scm (gcc-boot0)[arguments]: Adjust
configure-flag to also use '--with-long-double-128' on powerpc-linux.
---
 gnu/packages/commencement.scm | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

I ran into build troubles without this patch. It only affects gcc-boot0
on powerpc, everything else I built out to mesa was unaffected by a
64-bit/128-bit mismatch.

diff --git a/gnu/packages/commencement.scm b/gnu/packages/commencement.scm
index 36ebcee163..93caaac709 100644
--- a/gnu/packages/commencement.scm
+++ b/gnu/packages/commencement.scm
@@ -2733,12 +2733,12 @@ exec " gcc "/bin/" program
"--disable-shared"
"--enable-languages=c,c++"
 
-   ;; boot-triplet inserts "guix" in the triplet.
-   ,@(if (equal? "powerpc64le-guix-linux-gnu" 
(boot-triplet))
- ;; On POWER9 (little endian) glibc needs the
- ;; 128-bit long double type.
- '("--with-long-double-128")
- '())
+   ;; On POWER9 (little endian) glibc needs the 128-bit
+   ;; long double type.  32-bit PPC is affected by the
+   ;; changes applied for powerpc64le.
+   ,@(if (string-prefix? "powerpc" (boot-triplet))
+   '("--with-long-double-128")
+   '())
 
;; libstdc++ cannot be built at this stage
;; ("Link tests are not allowed after
-- 
2.31.1

[PATCH v2 02/12] gnu: guile-3.0: Fix building on powerpc-linux.

* gnu/packages/guile.scm (guile-3.0)[arguments]: On powerpc add two
phases to adjust for 32-bit big-endian systems.
---
 gnu/packages/guile.scm | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

This patch has the comments updated. I reported my findings to the guile
debbugs email and Other People™ are testing the changes on the other
architectures. Otherwise patch unchanged.

diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
index f63322794d..08ad50f7b0 100644
--- a/gnu/packages/guile.scm
+++ b/gnu/packages/guile.scm
@@ -305,7 +305,22 @@ without requiring the source code to be rewritten.")
  (substitute-keyword-arguments (package-arguments guile-2.2)
((#:configure-flags flags ''())
 `(cons "--disable-jit" ,flags)))
- (package-arguments guile-2.2)))
+ (if (string-prefix? "powerpc-" (%current-system))
+   (substitute-keyword-arguments (package-arguments guile-2.2)
+ ((#:phases phases)
+  `(modify-phases ,phases
+ (add-after 'unpack 'adjust-bootstrap-flags
+   (lambda _
+ ;; Upstream knows about suggested solution.
+ ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=45214
+ (substitute* "bootstrap/Makefile.in"
+   (("^GUILE_OPTIMIZATIONS.*")
+"GUILE_OPTIMIZATIONS = -O1 -Oresolve-primitives 
-Ocps\n"
+ (add-after 'unpack 'remove-failing-tests
+   (lambda _
+ ;; TODO: Discover why this test fails on powerpc-linux
+ (delete-file 
"test-suite/standalone/test-out-of-memory"))
+   (package-arguments guile-2.2
 (native-search-paths
  (list (search-path-specification
 (variable "GUILE_LOAD_PATH")
-- 
2.31.1

[PATCH v2 00/12] Add 32-bit powerpc support

This looks to be about 2 weeks worth of time since the last email, with
about 10 days of continuous building and is based on commit
76fc36d0a7215979bb74c05840f5a4de4ab5ea93 which changes the default gcc
to 8. I'll inline my comments in the top of each of the patches.

Some of the patches are going straight to core-updates but I've included
them anyway since the patchset is available in the official repo in the
wip-ppc branch and that's where I developed them.

Efraim Flashner (12):
  gnu: bootstrap: Add support for powerpc-linux.
  gnu: guile-3.0: Fix building on powerpc-linux.
  gnu: gcc-boot0: Use 128-bit long-double on powerpc-linux.
  gnu: binutils: Fix bug in test suite in libiberty.
  gnu: mesa: Add support for powerpc-linux.
  gnu: Add mac-fdisk.
  gnu: american-fuzzy-lop: Add support for powerpc-linux.
  build: qemu-command: Add support for powerpc.
  gnu: mercurial: Skip tests on powerpc-linux.
  gnu: nss: Skip tests on powerpc-linux.
  gnu: zstd: Adjust test suite for 32-bit architectures.
  gnu: glib: Disable failing test.

 gnu/build/vm.scm  |1 +
 gnu/local.mk  |4 +
 gnu/packages/base.scm |3 +-
 gnu/packages/bootstrap.scm|   37 +-
 gnu/packages/commencement.scm |   12 +-
 gnu/packages/compression.scm  |4 +-
 gnu/packages/debug.scm|2 +
 gnu/packages/disk.scm |   43 +
 gnu/packages/gl.scm   |   33 +-
 gnu/packages/glib.scm |   16 +-
 gnu/packages/guile.scm|   17 +-
 gnu/packages/nss.scm  |7 +-
 .../binutils-libiberty-endianness-bug.patch   |   36 +
 .../patches/glib-skip-failing-test.patch  |   27 +
 .../patches/mac-fdisk-gentoo-patchset.patch   |  866 +++
 gnu/packages/patches/mac-fdisk-p18.patch  | 2070 +
 gnu/packages/version-control.scm  |6 +-
 guix/packages.scm |4 +-
 m4/guix.m4|4 +-
 19 files changed, 3157 insertions(+), 35 deletions(-)
 create mode 100644 gnu/packages/patches/binutils-libiberty-endianness-bug.patch
 create mode 100644 gnu/packages/patches/glib-skip-failing-test.patch
 create mode 100644 gnu/packages/patches/mac-fdisk-gentoo-patchset.patch
 create mode 100644 gnu/packages/patches/mac-fdisk-p18.patch


base-commit: f08b070019a3c1697bb0b4a783dcd4f31243715a
prerequisite-patch-id: b98fc3a62ea8cceddca93361a4621026cebdb8e9
prerequisite-patch-id: 349889c70ebae8711909e6dfc0329235fee1a319
prerequisite-patch-id: dd3b6fe2e61b8588333468e597efec90314e482e
prerequisite-patch-id: 95964df281b3feb46795c5875c1091136fb9
prerequisite-patch-id: c150ac2fb2988685f28d788f4924b74a6264dce7
prerequisite-patch-id: a08c6f0dd727f598ef1258bb4233cfcec78913ee
prerequisite-patch-id: e36e1a27ce5055e53d5638f7139b4cf8c1ee68bf
prerequisite-patch-id: e59cdf3bd91cdba9e39d79d32c650eefea1749fd
prerequisite-patch-id: 2a1b37ff2e9b6f6787f4f3262d71cc26a005ccab
prerequisite-patch-id: 5cf027408739a4b2661cdf2709abdb8f1f06ce81
prerequisite-patch-id: 6deb5fd40689a243b1cce2c2fa5cca298bce253b
prerequisite-patch-id: 4c113e2088ef2674d35d985024c2a41daa8c679c
prerequisite-patch-id: edd89720757f9d3cedab3f3832abb4a8ec8bd83f
prerequisite-patch-id: 01258be2f32995622720bbc2eac1874282033604
prerequisite-patch-id: ed2fbb545f52e3ae813dc1b27e91e7ce2c84bce7
prerequisite-patch-id: 3523cc33853e2ff01d7b2916f5c481b052a674b5
prerequisite-patch-id: 92482c9dff576aa675e96a15bd02bd7471be517b
prerequisite-patch-id: f51d60f4622d9badefacdbb419d26c92a9387286
prerequisite-patch-id: dd2f7be90323949247ee684840ceea90ab2aed30
prerequisite-patch-id: f5f2f3cad462eb3c42df531f4b7fb0b2dc11959d
prerequisite-patch-id: ca605868541fdc9d2521c9bad0135a7a6e0b25b0
prerequisite-patch-id: 5ea5961816f5e60b38dbbacc6986860585d5b5c4
prerequisite-patch-id: 6d1cf903247a8ba700eb7b160b58d9b30b012a85
prerequisite-patch-id: 9d2c5115e7e6d19a06019cce74b58d036bc8f0d8
prerequisite-patch-id: b3699959ce306c85c4ed4756746e5273044f175a
prerequisite-patch-id: c587c3ca1d6bc31b1ff7f242066a8dd1603d3268
prerequisite-patch-id: 8fb15f1844dfe713c745b8095750912e03449d64
prerequisite-patch-id: fa64eb7a3a6312c0351e3452ad0300014a1cb2a0
prerequisite-patch-id: 84d9c39b89b986e096677904a5328b34e0072d30
prerequisite-patch-id: 0a0b1370f74ade7fcb989bd7e35520e9adac625e
prerequisite-patch-id: db9f94813d400e6734a1c80e40ad90aa7c1dd195
prerequisite-patch-id: db6b99cbc6e931613643690b3f72b41d73acf1b7
prerequisite-patch-id: 8441973a4f8c0f1713939fcd489274b7d30f44e4
prerequisite-patch-id: 9dd81d9a919ec6ca7f4bf07affdb7606f216cf44
prerequisite-patch-id: 88e793b3c2f41f4736b0867aaa5359a1a2f5df8f
prerequisite-patch-id: 7182f001b5ebaa7aafccf5d4c7e02e086ac6d25a
prerequisite-patch-id: fc8ca96ac1c7eb95a9326b2c2489f767c3f3184d
prerequisite-patch-id: 317a9f00e31e3a5a85542139bb70bfdadb9d7839
prerequisite-patch-id:

[PATCH v2 01/12] gnu: bootstrap: Add support for powerpc-linux.

On 923bb70a1bff657125c3008f119a477e5cb57c2b
   gnu:glibc-for-bootstrap: Fix patch.

Run
./pre-inst-env guix build --target=powerpc-linux-gnu bootstrap-tarballs

Producing

/gnu/store/dyj1wvayyp1ihaknkxniz1xamcf4yrhl-bootstrap-tarballs-0

With guix hash -rx 
/gnu/store/dyj1wvayyp1ihaknkxniz1xamcf4yrhl-bootstrap-tarballs-0

02xx2ydj28pwv3vflqffinpq1icj09gzi9icm8j4bwc4lca9irxn

* gnu/packages/bootstrap.scm (%bootstrap-executables): Add entries for
powerpc-linux.
(%bootstrap-guile-hash, %bootstrap-coreutils, %bootstrap-binutils,
%bootstrap-glibc, %bootstrap-gcc): Add entry for powerpc-linux.
* gnu/packages.scm (%supported-systems): Add powerpc-linux.
(%hydra-supported-systems): Remove powerpc-linux.
* m4/guix.m4: Add powerpc-linux as a supported system.
---
 gnu/packages/bootstrap.scm | 37 -
 guix/packages.scm  |  4 ++--
 m4/guix.m4 |  4 ++--
 3 files changed, 40 insertions(+), 5 deletions(-)

This patch is unchanged.

diff --git a/gnu/packages/bootstrap.scm b/gnu/packages/bootstrap.scm
index c8844a40a8..5a8028a465 100644
--- a/gnu/packages/bootstrap.scm
+++ b/gnu/packages/bootstrap.scm
@@ -91,6 +91,15 @@
   ,(base32 "1j51gv08sfg277yxj73xd564wjq3f8xwd6s9rbcg8v9gms47m4cx"))
  ("xz"
   ,(base32 "1d779rwsrasphg5g3r37qppcqy3p7ay1jb1y83w7x4i3qsc7zjy2")))
+("powerpc-linux"
+ ("bash"
+  ,(base32 "0hwlw5lcyjzadprf5fm0cv4zb6jw667g9amnmhq0lbixasy7j72j"))
+ ("mkdir"
+  ,(base32 "12lfwh5p8pp06250wgi9mdvjv1jdfpd5xpmvfc0616aj0xqh09hp"))
+ ("tar"
+  ,(base32 "00sbmwl8qh6alxv9mw4hvj1j4yipwmw5mrw6qad8bi2pr7ya5386"))
+ ("xz"
+  ,(base32 "0hi47y6zh5zz137i59l5ibw92x6g54zn7ris1b1ym9rvavsasg7b")))
 ("armhf-linux"
  ("bash"
   ,(base32 "0s6f1s26g4dsrrkl39zblvwpxmbzi6n9mgqf6vxsqz42gik6bgyn"))
@@ -141,6 +150,7 @@
   ;; This is where the bootstrap executables come from.
   '("https://git.savannah.gnu.org/cgit/guix.git/plain/gnu/packages/bootstrap/;
 "https://alpha.gnu.org/gnu/guix/bootstrap/;
+"http://flashner.co.il/guix/bootstrap/;
 "http://lilypond.org/janneke/guix/;))
 
 (define (bootstrap-executable-file-name system program)
@@ -148,6 +158,7 @@
   (match system
 ("powerpc64le-linux" (string-append system "/20210106/" program))
 ("i586-gnu" (string-append system "/20200326/" program))
+("powerpc-linux" (string-append system "/20200923/bin/" program))
 (_ (string-append system "/" program
   "?id=44f07d1dc6806e97c4e9ee3e6be883cc59dc666e"
 
@@ -343,6 +354,8 @@ or false to signal an error."
  (match system
("aarch64-linux"
 "/20170217/guile-2.0.14.tar.xz")
+   ("powerpc-linux"
+"/20200923/guile-2.0.14.tar.xz")
("armhf-linux"
 "/20150101/guile-2.0.11.tar.xz")
("i586-gnu"
@@ -368,7 +381,9 @@ or false to signal an error."
 ("aarch64-linux"
  (base32 "1giy2aprjmn5fp9c4s9r125fljw4wv6ixy5739i5bffw4jgr0f9r"))
 ("i586-gnu"
- (base32 "0wgqpsmvg25rnqn49ap7kwd2qxccd8dr4lllzp7i3rjvgav27vac"
+ (base32 "0wgqpsmvg25rnqn49ap7kwd2qxccd8dr4lllzp7i3rjvgav27vac"))
+("powerpc-linux"
+ (base32 "1by2p7s27fbyjzfkcw8h65h4kkqh7d23kv4sgg5jppjn2qx7swq4"
 
 (define (bootstrap-guile-origin system)
   "Return an  object for the Guile tarball of SYSTEM."
@@ -501,6 +516,8 @@ $out/bin/guile --version~%"
  
"/20210106/static-binaries-0-powerpc64le-linux-gnu.tar.xz")
 ("i586-gnu"
  
"/20200326/static-binaries-0-i586-pc-gnu.tar.xz")
+("powerpc-linux"
+ 
"/20200923/static-binaries.tar.xz")
 (_
  
"/20131110/static-binaries.tar.xz")))
  %bootstrap-base-urls))
@@ -524,6 +541,9 @@ $out/bin/guile --version~%"
   ("i586-gnu"
(base32
 
"17kllqnf3fg79gzy9ansgi801c46yh9c23h4d923plvb0nfm1cfn"))
+  ("powerpc-linux"
+   (base32
+
"0kspxy0yczan2vlih6aa9hailr2inz000fqa0gn5x9d1fxxa5y8m"))
   ("mips64el-linux"
(base32
 
"072y4wyfsj1bs80r6vbybbafy8ya4vfy7qj25dklwk97m6g71753"))
@@ -574,6 +594,8 @@ $out/bin/guile --version~%"
  
"/20210106/binutils-static-stripped-2.34-powerpc64le-linux-gnu.tar.xz")
 ("i586-gnu"
  
"/20200326/binutils-static-stripped-2.34-i586-pc-gnu.tar.xz")
+

Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.