Re: go-build-system possible improvments

[Maxim Cournoyer]

Hi François,

François  writes:

> Hello Maxim,
>
> On Tue, Apr 20, 2021 at 10:00:15PM -0400, Maxim Cournoyer wrote:
>> That's exactly what we're doing now (disable Go module with
>> GO11MODULE=off and use GOPATH to find the sources).  Debian does
>> something lazy like just calling 'go module vendor', which builds a huge
>> directory filled with all the sources needed.  So does Nix.  Only Gentoo
>> goes to some length, from what I had seen last I check.
>
> Do you have some links to share on how Gentoo does it?

I think I ended up looking in their git repo, but otherwise there's this
page for their go-module.eclass module:
https://devmanual.gentoo.org/eclass-reference/go-module.eclass/index.html.

HTH,

Maxim

Re: Organizing Tui Apps

[Ryan Prior]

On April 23, 2021, Leo Famulari  wrote:
> On Fri, Apr 23, 2021 at 10:00:14PM +0200, Leo Prikler wrote:
> > Spreadsheets sounds fine to me, but I think the most important ones
> > (libreoffice and org-mode) are already excluded from that module for
> > obvious reasons ;)
> > Perhaps an even more generic "office" module might be better,
> because
> > then we could at least add some small word processors to it, WDYT?
> Or
> > maybe sc-im already fits as-is into textutils if you squint hard
> enough
>
> My main objection was about moving packages. So whatever module you
> create is fine, but let's not start moving libreoffice and org-mode
> around.

As the person who originally suggested organizing tui-apps into a file,
I'd envisioned it as being sort of like its own desktop environment. So
similar to how we might put Gnome (GTK) apps and KDE (Qt) apps together,
we might put tui (ASCII terminal) apps together.

I am not upset if that's unconvincing to others & will happily drop the
proposal if it's not useful, I never intended for it to become a drawn
out discussion. It also makes sense to me that we might organize things
by programming language or by function, those each have strengths as
strategies.

Ryan

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

[Raghav Gururajan]

Hi Maxim!


Oh, indeed, sorry for the confusion.  I think I got tricked by seeing
the changelog for 1.17.2 under their releases/ directory
(https://www.cairographics.org/releases/ChangeLog.cairo-1.17.2).


No worries! I was confused by that too, while I was working on cairo 
package.


Regards,
RG.



OpenPGP_signature
Description: OpenPGP digital signature

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

[Christopher Baines]

Luciana Lima Brito  writes:

> Hi,
>
> On Thu, 22 Apr 2021 22:15:34 +0100
> Christopher Baines  wrote:
>
> I'm stuck.
>
>> 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?
>
> It is not so clear to me. I tried three ways, always testing on
> "outputs":
>
> First, I tried using an alist  within
> the map function on derivation-outputs-differences-data
> (comparison.scm), and I applied a list->vector on the result. It
> worked, but I do believe that this is not the way we want it to be.
> This way the label comes as another field, and I got the result like
> this:
>
> code:
>
> `((label . ,(cond
>   ((and (memq base-derivation-id
>   parsed-derivation-ids)
> (memq target-derivation-id
>   parsed-derivation-ids))
>'common)
>   ((memq base-derivation-id parsed-derivation-ids)
>  'base)
>   (else 'target)))
>(name . ,output-name)
>(path . ,path)
>,@(if (string? hash-algorithm)
>  `((hash-algorithm . ,hash-algorithm))
>  '())
>,@(if (string? hash)
>  `((hash . ,hash))
>  '())
>(recursive . ,(string=? recursive "t")))
>
> json outputs:
>0:
>   label: "base"
>   name: "foo"
>   path: "bar"
>   recursive: #f
>
> This way I only used the function derivation-outputs-differences-data,
> not using group-to-alist or group-by-last-element.
>
> The second way I tried was to pass an alist as first element of the list
> and append the labels (base or/and target):
>
> (list
>  `((name . ,output-name)
>(path . ,path)
>,(if (string? hash-algorithm)
>`(hash-algorithm . ,hash-algorithm)
>'())
>,(if (string? hash)
>`(hash . ,hash)
>'())
>(recursive . ,(string=? recursive "t")))
>
>  (append (if (memq base-derivation-id
>parsed-derivation-ids)
>  '(base)
>  '())
>  (if (memq target-derivation-id
>parsed-derivation-ids)
>  '(target)
>  '(
>
> But this seemed to result in an extra parenthesis in the result, which
> didn't work.

I'd expect ,@(if ... rather than just ,(if ... and then having a list of
a pair when you want the if to result in something. There's some
documentation on quasiquoting (the ` thing) here:

  
https://www.gnu.org/software/guile/manual/html_node/Expression-Syntax.html#index-quasiquote

> So I tried a third way, using the pairs
> individually in the list and the append.
>
> (list
>  `(name . ,output-name)
>  `(path . ,path)
>  (if (string? hash-algorithm)
>  `(hash-algorithm . ,hash-algorithm)
>  '())
>  (if (string? hash)
>  `(hash . ,hash)
>  '())
>  `(recursive . ,(string=? recursive "t"))
>
>  (append (if (memq base-derivation-id
>parsed-derivation-ids)
>  '(base)
>  '())
>  (if (memq target-derivation-id
>parsed-derivation-ids)
>  '(target)
>  '(
>
>
> Furthermore, all these methods would require tweaking the html
> processing.
>
> Here is an idea I just had. The first way I tried returned a
> vector. This worked but kind of ugly. As I am already
> getting the alist, I just thought about writing another function to
> group by the first elements (the labels), something like this:
>
> (group-by-first-element l)
>
> where l is the alist I get from the first method I mentioned.
> And then, I would have the following alist:
> ((base (...))
>  (target (...))
>  (common ((...)(...)(...
>
> which is the current alist we are using in controller.scm.
> Problem is, I still don't know how to do this, and this seems somewhat
> too long winded to get the proper alist.

So, the current code copes with the general case where there can be any
number of outputs, and some can be common between two derivations, and
some other number can come from just the base or target derivation.

The query will effectively return a list with an element for each
output.

I'd perhaps try to work within the current structure at first at least.

If you change derivation-outputs-differences-data to return a list of
alists rather than a list of lists, I'd look at adapting the
group-to-alist call to 

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

[Luciana Lima Brito]

Hi,

On Thu, 22 Apr 2021 22:15:34 +0100
Christopher Baines  wrote:

I'm stuck.
 
> 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?

It is not so clear to me. I tried three ways, always testing on
"outputs":

First, I tried using an alist  within
the map function on derivation-outputs-differences-data
(comparison.scm), and I applied a list->vector on the result. It
worked, but I do believe that this is not the way we want it to be.
This way the label comes as another field, and I got the result like
this:

code:

`((label . ,(cond
((and (memq base-derivation-id
parsed-derivation-ids)
  (memq target-derivation-id
parsed-derivation-ids))
   'common)
((memq base-derivation-id parsed-derivation-ids)
   'base)
(else 'target)))
   (name . ,output-name)
   (path . ,path)
   ,@(if (string? hash-algorithm)
 `((hash-algorithm . ,hash-algorithm))
 '())
   ,@(if (string? hash)
 `((hash . ,hash))
 '())
   (recursive . ,(string=? recursive "t")))

json outputs:
   0:
label: "base"
name: "foo"
path: "bar"
recursive: #f

This way I only used the function derivation-outputs-differences-data,
not using group-to-alist or group-by-last-element.

The second way I tried was to pass an alist as first element of the list
and append the labels (base or/and target):

(list
 `((name . ,output-name)
   (path . ,path)
   ,(if (string? hash-algorithm)
   `(hash-algorithm . ,hash-algorithm)
   '())
   ,(if (string? hash)
   `(hash . ,hash)
   '())
   (recursive . ,(string=? recursive "t")))
 
 (append (if (memq base-derivation-id
   parsed-derivation-ids)
 '(base)
 '())
 (if (memq target-derivation-id
   parsed-derivation-ids)
 '(target)
 '(

But this seemed to result in an extra parenthesis in the result, which
didn't work.

So I tried a third way, using the pairs
individually in the list and the append.

(list
 `(name . ,output-name)
 `(path . ,path)
 (if (string? hash-algorithm)
 `(hash-algorithm . ,hash-algorithm)
 '())
 (if (string? hash)
 `(hash . ,hash)
 '())
 `(recursive . ,(string=? recursive "t"))
 
 (append (if (memq base-derivation-id
   parsed-derivation-ids)
 '(base)
 '())
 (if (memq target-derivation-id
   parsed-derivation-ids)
 '(target)
 '(


Furthermore, all these methods would require tweaking the html
processing.

Here is an idea I just had. The first way I tried returned a
vector. This worked but kind of ugly. As I am already
getting the alist, I just thought about writing another function to
group by the first elements (the labels), something like this:

(group-by-first-element l)

where l is the alist I get from the first method I mentioned.
And then, I would have the following alist:
((base (...))
 (target (...))
 (common ((...)(...)(...

which is the current alist we are using in controller.scm.
Problem is, I still don't know how to do this, and this seems somewhat
too long winded to get the proper alist.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

Re: Organizing Tui Apps

[Leo Famulari]

On Fri, Apr 23, 2021 at 10:00:14PM +0200, Leo Prikler wrote:
> Spreadsheets sounds fine to me, but I think the most important ones
> (libreoffice and org-mode) are already excluded from that module for
> obvious reasons ;)
> Perhaps an even more generic "office" module might be better, because
> then we could at least add some small word processors to it, WDYT?  Or
> maybe sc-im already fits as-is into textutils if you squint hard enough

My main objection was about moving packages. So whatever module you
create is fine, but let's not start moving libreoffice and org-mode
around.

Re: Security related tooling project

[Christopher Baines]

Ludovic Courtès  writes:

> Hello Chris!
>
> Christopher Baines  skribis:
>
>> In May last year (2020), I submitted an application to NLNet. The work I
>> set out wasn't something I was doing at the time, but something I hadn't
>> yet found time to work on, tooling specifically around security issues.
>>
>> The application got a bit lost, probably somewhat down to email issues
>> on my end. Anyway, things picked up again in February of this year
>> (2021), and this is now something I'm looking to do roughly over the
>> next 8 months.
>
> I’m late to the party, but I think this is excellent news!  Well done!
>
>> 1: https://git.cbaines.net/guix/tooling-to-improve-security-and-trust/about/
>
> [...]
n>
>> In terms of looking at security from a project perspective, I'm thinking
>> about these kinds of needs/questions:
>>
>>  - What security issues affect this revision of Guix? (latest or otherwise)
>>
>>  - How do Guix contributors find out about new security issues that
>>affect Guix revisions they're interested in?
>>
>> From the user perspective, I want to look at things like:
>>
>>  - How do I find out what (if any) security issues affect the software
>>I'm currently running (through Guix)?
>>
>>  - How can I get notified when a new security issue affects the software
>>I'm currently running (through Guix)?
>
> That sounds like a great plan!
>
> I see several “intermediate” issues that would be super helpful for the
> overall project, such as better CPE matching as Léo suggested and/or
> providing CPE suggestions: .
>
> I think the Data Service is in a great position to help out
> wrt. monitoring.  I think it’d be nice to architect things in a way that
> services enhance monitoring, but are not required for get proper
> monitoring.  For instance, the proposed ‘guix health’¹ can be
> implemented without relying on intermediate services at all (it still
> needs to rely on the NIST server, of course, but we don’t need extra
> services.)
>
> Anyhow, it’s awesome to see you work in this area.  Like Chris Marusich
> wrote, Guix is in a good position to address security issues, and you’re
> obviously in a very good position to know what and how to improve the
> state of things in Guix, so all hail!

That's good to hear!

Thanks,

Chris


signature.asc
Description: PGP signature

Re: Security related tooling project

[Christopher Baines]

Bengt Richter  writes:

> Given that crims  monitor developer discussions to discover
> unfixed vulnerabilities and clues re exploiting them,
> what are your ideas to avoid building a tool that can be abused?
>
> E.g., How will your tool avoid leaking info during an embargo window
> while trusted developers are secretly/privately fixing
> critical vulns?

That's a point to consider I think. Most of what I'm thinking about is
for published vulnerabilities in software packaged for Guix, but you
raise a valid point, so thanks for bringing it up.

Chris


signature.asc
Description: PGP signature

Re: Organizing Tui Apps

[Leo Prikler]

Am Freitag, den 23.04.2021, 15:04 + schrieb jgart:
> This is a continuation of the thread started at 
> http://issues.guix.gnu.org/47852
> 
> Ekaitz: What do we want to be the main point of the programs: their
> UI or their goal?
> 
> jgart: I would prefer for them to be catalogued based on their
> intended usage or goal, as Ekaitz
> suggested.
> 
> What do people think?
> 
> For sc-im I last sent a patch that puts it in a new file/module
> called spreadsheets.
> 
> Is everyone happy with that suggestion from Leo? 
> 
> I think it is a good idea and seems to agree with what we were
> leaning towards in the conversation.
> 
> I guess now we just need to put more spreadsheet programs into the
> spreadsheets module to justify it :slight_smile:
Spreadsheets sounds fine to me, but I think the most important ones
(libreoffice and org-mode) are already excluded from that module for
obvious reasons ;)
Perhaps an even more generic "office" module might be better, because
then we could at least add some small word processors to it, WDYT?  Or
maybe sc-im already fits as-is into textutils if you squint hard enough
.

I don't think, we should have a tui-apps module.  How would you feel
about a gui-apps module, that has all of GNOME, KDE and some more?  Not
really good, no?  How about cli for everything, that has a CLI? 
Doesn't seem to make much sense.

Regards,
Leo

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

[Leo Famulari]

On Fri, Apr 23, 2021 at 09:33:07PM +0200, Léo Le Bouter wrote:
> I knew about this but I didnt feel like telling Raghav to do yet
> another rebase. I felt like Raghav was taking on with so much already.
> The rebase was specially complicated because Raghav's commit changed
> indentation, git has bad quite bad UX for cases like these. At the time
> I had lots of things to handle also and couldnt spend lots of time on
> it myself. I didnt feel like blocking the merge of these patches for
> commit history was worth it at all. Such blocking could have hindered
> the GNOME upgrade effort even more. Thankfully now there's lots of
> energy being put to it, at the time there wasnt anyone else than Raghav
> and me.

I'm sympathetic. There is an imbalance between the work that we want to
complete, and the time and energy that we can give to it.

And in the case of GNOME, we have already fallen short of our goals
several times, having missed multiple upgrades.

I too have felt the temptation to cut corners with Git when I know that
the final result will be "okay". But Guix is not just about the final
product (a release, or a merge). We also have the --commit option to
Guix commands, and `guix time-machine`. So the Git history is important
too.

And I have also spent several hours at a time, focused on completing
(after several restarts) a complicated rebase involving dozens of
commits. And I've done that many times.

I do think that Mark is being hyperbolic about the wip-gnome branch. The
name says "work in progress" and we don't hold those branches to a high
standard. But what happened on core-updates *must not happen again*.

For a task as large as "updating GNOME in Guix", history tells me that
it has to be a group effort. In many cases, the hardest part of a
project is coordination and leadership, not coding. I hope that this
current effort continues, and that more people decide to join.


signature.asc
Description: PGP signature

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

[Léo Le Bouter]

On Fri, 2021-04-23 at 15:18 -0400, Leo Famulari wrote:
> Léo and Raghav, you need to keep learning our workflow around
> security
> updates.  It's not okay to remove security patches and later update a
> package to a fixed version in a different commit. `git rebase` is the
> tool to learn for cases like this one.

I knew about this but I didnt feel like telling Raghav to do yet
another rebase. I felt like Raghav was taking on with so much already.
The rebase was specially complicated because Raghav's commit changed
indentation, git has bad quite bad UX for cases like these. At the time
I had lots of things to handle also and couldnt spend lots of time on
it myself. I didnt feel like blocking the merge of these patches for
commit history was worth it at all. Such blocking could have hindered
the GNOME upgrade effort even more. Thankfully now there's lots of
energy being put to it, at the time there wasnt anyone else than Raghav
and me.

Léo


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

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

[Leo Famulari]

On Fri, Apr 23, 2021 at 08:50:37PM +0200, Léo Le Bouter wrote:
> I think there is no problem in accepting criticism but there is a
> certain way Mark presents criticism and I don't feel like I can respond
> to it when it is written in such way. Over several emails Mark was
> looking to point to people who were somehow responsible for whatever
> "damage" for changes that happened on a branch nobody uses and always
> contains ongoing work (core-updates), so maintaining it security-wise
> is not as much of a question. The result is that we have a long thread
> of people responding etc. causing a fuss over something that just needs
> to be fixed rather than find whoever is somehow "responsible". I feel
> like we're collectively responsible. We try our best at all times,
> during this GNOME upgrade I also tried to take into account Raghav's
> feelings so they do not give up and have a rewarding review experience,
> I knew these commits werent great, I have written about it here: <
> https://issues.guix.gnu.org/42958#67>.

I have to agree with everybody in this thead.

The commits in question were problematic (especially on core-updates,
which is not a "WIP" branch and thus cannot be rewritten to fix past
problems). I'm not confident that the security fixes would have been
reinstated on core-updates if Mark had not asked about them.

Léo and Raghav, you need to keep learning our workflow around security
updates.  It's not okay to remove security patches and later update a
package to a fixed version in a different commit. `git rebase` is the
tool to learn for cases like this one.

However, Mark, you have way more experience, and you need to handle
these things differently. If you don't trust certain Guix contributors,
take it up with the maintainers — in private. The style of communication
you used here is ineffective and will dissuade people from contributing
to Guix. Do you want Léo and Raghav to learn and improve? Or do you want
them to leave? Remember that we all begin as beginners.


signature.asc
Description: PGP signature

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

[Leo Prikler]

Hi,

Am Freitag, den 23.04.2021, 20:50 +0200 schrieb Léo Le Bouter:
> I think there is no problem in accepting criticism but there is a
> certain way Mark presents criticism and I don't feel like I can
> respond
> to it when it is written in such way. Over several emails Mark was
> looking to point to people who were somehow responsible for whatever
> "damage" for changes that happened on a branch nobody uses and always
> contains ongoing work (core-updates), so maintaining it security-wise
> is not as much of a question. The result is that we have a long
> thread
> of people responding etc. causing a fuss over something that just
> needs
> to be fixed rather than find whoever is somehow "responsible". 
I disagree with the sentiment, that core-updates is fair game for any
kind of commit.  Now, naturally, since they cause many rebuilds it may
be harder to verify that upgrading some packages does not lead to
failure in another (especially without the CI), contributing to the
"work in progress" nature of core-updates, but this still doesn't
excuse removing security fixes.  
We all expect, that at some point we can merge core-updates "as is"
into master and commits like that call this assumption into question,
instead demanding a full review of a branch, whose patches should
already have been reviewed by the time they land.

> I feel
> like we're collectively responsible. We try our best at all times,
> during this GNOME upgrade I also tried to take into account Raghav's
> feelings so they do not give up and have a rewarding review
> experience,
> I knew these commits werent great, I have written about it here: <
> https://issues.guix.gnu.org/42958#67>;.
I think a more rewarding experience would have been to help them arrive
at a point, where such changes are no longer needed for the rest of
their patch set.  Not only would this have solved their immediate
issue, it would also have been a good learning experience and we
wouldn't need to discuss this at lengths several months later.

I have worked with Raghav before on telegram-desktop (and other
packages as well) and they were pretty patient with about 20 versions
being sent back and forth between us until we arrived at a set of
descriptions, that we could safely push.  Not nearly as many versions
would need to be sent in the case of a "cosmetic changes" patch, when I
ported their GStreamer updates to staging, I noticed that it was mostly
the indentation, that would screw things up for future patches.  I
admit, sometimes Raghav appears to "just want to get the job done
quickly", but giving in to such urges helps no one.

Regards,
Leo

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

[Léo Le Bouter]

On Fri, 2021-04-23 at 13:52 -0400, Maxim Cournoyer wrote:
> Actually, there *is* a "new" stable release available on their
> release
> page, 1.17.2 [0]
> 
> According to NVD [1], that latest version has no known CVE [1].
> 
> Léo, could it be that you had planned to do this update, but it
> somehow
> fell into the cracks?  In any case I agree with the others that it'd
> have been better to ungraft/remove patches in the same commit that
> updates the software to a version that incorporates the fixes, as I'm
> sure you already know: it'd have prevented this kind of situation.

Considering the GNOME upgrade is not finished yet, this is indeed
ongoing work. I would've never done this on master.

> 
> I also urge you to remain calm and collaborative even in the face of
> criticism; as Ricardo said, escalating things will lead us nowhere
> good.
> Honest mistakes are made and that's no problem so long as we stand
> ready
> to apologize for them and work together for a resolution.
> 

I think there is no problem in accepting criticism but there is a
certain way Mark presents criticism and I don't feel like I can respond
to it when it is written in such way. Over several emails Mark was
looking to point to people who were somehow responsible for whatever
"damage" for changes that happened on a branch nobody uses and always
contains ongoing work (core-updates), so maintaining it security-wise
is not as much of a question. The result is that we have a long thread
of people responding etc. causing a fuss over something that just needs
to be fixed rather than find whoever is somehow "responsible". I feel
like we're collectively responsible. We try our best at all times,
during this GNOME upgrade I also tried to take into account Raghav's
feelings so they do not give up and have a rewarding review experience,
I knew these commits werent great, I have written about it here: <
https://issues.guix.gnu.org/42958#67>.

> I see that 宋文武 has pushed a commit
> (2ab4f4c950ffa7ca40271a534cb3bed997672138) to core-updates
> reinstating
> the security patches; thanks!
> 

Great! Thanks for figuring this out.

> Thank you,
> 
> Maxim
> 
> [0]  https://www.cairographics.org/releases/
> [1]  
> https://nvd.nist.gov/vuln/search/results?form_type=Advanced_type=overview_type=all=cpe:2.3:a:cairographics:cairo:-:*:*:*:*:*:*:*

Léo


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

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

[Maxim Cournoyer]

Hello Raghav,

Raghav Gururajan  writes:

> Hi Maxim!
>
>> Actually, there *is* a "new" stable release available on their release
>> page, 1.17.2
>
> It seems 1.16.0 is the latest+stable version.
>
> Quoting their download, "Please download one of the latest
> [releases](https://cairographics.org/releases/) in order to get an 
> API-stable version of cairo."

Oh, indeed, sorry for the confusion.  I think I got tricked by seeing
the changelog for 1.17.2 under their releases/ directory
(https://www.cairographics.org/releases/ChangeLog.cairo-1.17.2).

Thank you,

Maxim

Re: go-build-system possible improvments

[François]

Hello Maxim,

On Tue, Apr 20, 2021 at 10:00:15PM -0400, Maxim Cournoyer wrote:
> That's exactly what we're doing now (disable Go module with
> GO11MODULE=off and use GOPATH to find the sources).  Debian does
> something lazy like just calling 'go module vendor', which builds a huge
> directory filled with all the sources needed.  So does Nix.  Only Gentoo
> goes to some length, from what I had seen last I check.

Do you have some links to share on how Gentoo does it?

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

[Pierre Neidhardt]

Hi,

A quick search got me this:

https://github.com/DalekBaldwin/on-lisp/blob/master/on-lisp.asd

So it seems that the license is an informal

> As long as you tell people it's modified and link to the original,
> that's fine.  --pg

Not sure what to make of it.

I've heard of a few libraries that implement some "On Lisp" algorithms,
but I suppose we would need to look in details.

Maybe ask on r/lisp or r/common_lisp.

-- 
Pierre Neidhardt
https://ambrevar.xyz/


signature.asc
Description: PGP signature

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

[Raghav Gururajan]

Hi Maxim!


Actually, there *is* a "new" stable release available on their release
page, 1.17.2


It seems 1.16.0 is the latest+stable version.

Quoting their download, "Please download one of the latest 
[releases](https://cairographics.org/releases/) in order to get an 
API-stable version of cairo."


Regards,
RG.



OpenPGP_signature
Description: OpenPGP digital signature

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

[Maxim Cournoyer]

Hi,

Mark H Weaver  writes:

> 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.

Actually, there *is* a "new" stable release available on their release
page, 1.17.2 [0]

According to NVD [1], that latest version has no known CVE [1].

Léo, could it be that you had planned to do this update, but it somehow
fell into the cracks?  In any case I agree with the others that it'd
have been better to ungraft/remove patches in the same commit that
updates the software to a version that incorporates the fixes, as I'm
sure you already know: it'd have prevented this kind of situation.

I also urge you to remain calm and collaborative even in the face of
criticism; as Ricardo said, escalating things will lead us nowhere good.
Honest mistakes are made and that's no problem so long as we stand ready
to apologize for them and work together for a resolution.

I see that 宋文武 has pushed a commit
(2ab4f4c950ffa7ca40271a534cb3bed997672138) to core-updates reinstating
the security patches; thanks!

Thank you,

Maxim

[0]  https://www.cairographics.org/releases/
[1]  
https://nvd.nist.gov/vuln/search/results?form_type=Advanced_type=overview_type=all=cpe:2.3:a:cairographics:cairo:-:*:*:*:*:*:*:*

Organizing Tui Apps

[jgart]

This is a continuation of the thread started at http://issues.guix.gnu.org/47852

Ekaitz: What do we want to be the main point of the programs: their UI or their 
goal?

jgart: I would prefer for them to be catalogued based on their intended usage 
or goal, as Ekaitz
suggested.

What do people think?

For sc-im I last sent a patch that puts it in a new file/module called 
spreadsheets.

Is everyone happy with that suggestion from Leo? 

I think it is a good idea and seems to agree with what we were leaning towards 
in the conversation.

I guess now we just need to put more spreadsheet programs into the spreadsheets 
module to justify it :slight_smile:

Re: What's the meaning of the percent sign in variable names

[Leo Prikler]

Am Freitag, den 23.04.2021, 14:22 + schrieb Luis Felipe:
> Hi,
> 
> Are all these constants (%base-packages, for example)? Is this a Guix
> convention or does it come from Guile? Although looking at Guile's
> Variable index I see many constants in uppercase, and also some
> variables prefixed with the percent sign, while Guix' Programming
> index doesn't seem to list any uppercase names.
The tendency to prepend variables with % certainly comes from Guile or
more generally Scheme.  The upper case constants in Guile likely come
from C, where this convention is more popular and IIUC often concerns
integer constants, that are in some way important for the C side of
things (think of stuff like OPEN_READ).  Since that is not a concern
for Guix, a package manager mostly written in Guile Scheme, we can
prefix our constants with a percent sign.

Regards,
Leo

What's the meaning of the percent sign in variable names

[Luis Felipe]

Hi,

Are all these constants (%base-packages, for example)? Is this a Guix 
convention or does it come from Guile? Although looking at Guile's Variable 
index I see many constants in uppercase, and also some variables prefixed with 
the percent sign, while Guix' Programming index doesn't seem to list any 
uppercase names.


Thanks,

---
Luis Felipe López Acevedo
https://luis-felipe.gitlab.io/

Re: bug#47867: [1.2.1 pre-release testing] substitute downloading and TLS errors

[pelzflorian (Florian Pelz)]

Success!  Thank you.
65;6003;1c
On Fri, Apr 23, 2021 at 11:19:28AM +0200, Ludovic Courtès wrote:
> Florian, could you try again with the attached patch?

It succeeds on two full installs of Enlightenment, no errors, no
prolonged getting stuck.


> If you have the courage, it would be awesome if you could also try the
> patch without the ‘error/again’ bits.

This fails.  I tried with

-(unless (false-if-networking-error
- (begin
-   (for-each (cut write-request <> buffer) batch)
-   (put-bytevector p (get))
-   (force-output p)
-   #t))
-  ;; If PORT becomes unusable, open a fresh connection and retry.
-  (close-port p)  ; close the broken port
-  (connect #f requests result)))
+;; Swallow networking errors that could occur due to connection reuse
+;; and the like; they will be handled down the road when trying to
+;; read responses.
+(false-if-networking-error
+ (begin
+   (for-each (cut write-request <> buffer) batch)
+   (put-bytevector p (get))
+   (force-output p


only and not the rest of your patch, on Guix git master where the full
patch had worked, it fails again with TLS errors (the same error of
Resource temporarily unavailable in procedure
'write_to_session_record_port') after downloading the enlightenment
substitute.


> I double-checked and the GnuTLS Guile bindings already
> handle GNUTLS_E_AGAIN and GNUTLS_E_INTERRUPTED, so my guess is that this
> was just a side effect of dealing with stale TLS sessions:
>   https://gitlab.com/gnutls/gnutls/-/blob/master/guile/src/core.c#L1042

Strange,.

Regards,
Florian

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

[Christopher Baines]

Canan Talayhan  writes:

> It seems after testing lots of pages this one escaped me since I only
> tested the working case.
>
> Please find the quick fix in the link below.
> https://pastebin.ubuntu.com/p/s7tWyPHZ8F/

Great, that fixes the issue with the revision comparison page.

> I'm looking forward to making another contribution. Could you please
> review it as soon as possible?

I've gone ahead and merged this. I made some changes to the indentation,
I've generally just left that to Emacs, so that's effectively the
indentation style currently. I also tweaked the wording in the commit
message.

As for what to do next, it would be good to start looking at some stuff
that's more related to the project topic.

Part of the revision processing that I believe is quite slow and
hopefully can be improved is populating the package_metadata table. The
relevant lines in the job output look something like this:

  debug: Starting querying the temp_package_metadata
  debug: Finished querying the temp_package_metadata, took 1902 seconds

That output comes from the with-time-logging bit around here [1]. It's a
single query, generated by temp-table-select-query which is taking
around 30 minutes it seems, and I'd hope either the query can be made
faster, or some other faster way of doing what needs doing here can be
found.

1: 
https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/utils.scm#n333

insert-missing-data-and-return-all-ids is used in a few places, but this
specific issue arises when called from here [2].

2: 
https://git.savannah.gnu.org/cgit/guix/data-service.git/tree/guix-data-service/model/package-metadata.scm#n373

Making inserting package metadata faster is the overall goal, but I'd
suggest first just trying to reproduce the slow query outside of the
revision processing process. That way you'll be able to look at what the
query is doing and quickly test changes.

The approach I'd recommend is, make yourself a realistic
temp_package_metadata table by populating it with all the
package_metadata entries for a single revision already in your local
database. Then construct and try the slow query, and see how long it
takes, and look at the query plan (run the query with EXPLAIN at the
start).

Do let me know if you have any questions or get stuck, I'll hopefully be
around on IRC, and if I don't respond within a few minutes there, just
email me.


signature.asc
Description: PGP signature

Re: bug#47867: [1.2.1 pre-release testing] substitute downloading and TLS errors

[Ludovic Courtès]

Hi Florian,

(Cc: Chris who’s also familiar with (guix http-client).)

"pelzflorian (Florian Pelz)"  skribis:

> It still gets stuck (sometimes with enlightenment, one time with
> udisks, restarting the install fixed it once).  After getting stuck,
> this different error message is shown now; no TLS error (copied by
> manual typing, there may be typos):
>
> gtk-doc-1.28  653KiB2.4MiB/s 00:00 [] 
> 100.0%
> udisks-2.8.4  842KiB1.6MiB/s 00:00 [] 
> 100.0%
>
> substitute: updating substitutes from 'https://ci.guix.gnu.org'... 
> 100.0%Backtrace:
> substitute: In ice-9/boot-9.scm:
> substitute:   1736:10 17 (with-exception-handler _ _ #:unwind? _ # _)
> substitute: In unknown file:
> substitute:   16 (apply-smob/0 #)
> substitute: In ice-9/boot-9.scm:
> substitute: 718:2 15 (call-with-prompt _ _ # default-prompt-handle…>)
> substitute: In ice-9/eval.scm:
> substitute: 619:8 14 (_ #(#(#)))
> substitute: In guix/ui.scm:
> substitute:   2164:12 13 (run-guix-command _ . _)
> substitute: In ice-9/boot-9.scm:
> substitute:   1736:10 12 (with-exception-handler _ _ #:unwind? _ # _)
> substitute:   1736:10 11 (with-exception-handler _ _ #:unwind? _ # _)
> substitute:   1731:15 10 (with-exception-handler _ _ #:unwind? _ # _)
> substitute: In guix/scripts/substitute.scm:
> substitute:745:18  9 (_)
> substitute:346:26  8 (process-query # _ #:cache-urls _ 
> #:acl _)
> substitute: In guix/substitutes.scm:
> substitute:358:27  7 (lookup-narinfos/diverse _ _ # 7fc2d3b15000 …>
> substitute:315:31  6 (lookup-narinfos _ _ #:open-connection _ # _)
> substitute:238:26  5 (fetch-narinfos _ _ #:open-connection _ # _)
> substitute: In ice-9/boot-9.scm:
> substitute:   1669:16  4 (raise-exception _ #:continuable? _)
> substitute:   1669:16  3 (raise-exception _ #:continuable? _)
> substitute:   1764:13  2 (_ #< _ components: 
> assertion-fail…>)
> substitute:   1669:16  1 (raise-exception _ #:continuable? _)
> substitute:   1669:16  0 (raise-exception _ #:continuable? _)
> substitute:
> substitute: In ice-9/boot-9.scm:1669:16 In procedure raise-exception:
> substitute: In procedure %read-line: Wrong type argument in position 1 
> (expecting open input port): #
> guix system: error: 
> `/gnu/store/k3n98i1fk9awd5ydv4ry4k4rlpp7i13m7-guix-1.2.0-22.c467718/bin/guix 
> substitute' died unexpectedly

I think I got it: commit 205833b72c5517915a47a50dbe28e7024dc74e57 (then
carried over in 45fce38fb0b6c6796906149ade145b8d3594c1c6) introduced a
call to ‘connect’ in non-tail position.  Once that recursive call to
‘connect’ had completed, ‘http-multiple-get’ would go on in ‘loop’
trying to re-process responses, but at that point there aren’t any
responses left to process.

This problem could only happen if a networking exception would occur
while sending HTTP requests for narinfos.  Thus, it was most likely to
occur when interleaving substitutions and queries, as in the snippet you
provided above, because then ‘http-multiple-get’ was more likely to be
passed a stale reused connection.

Florian, could you try again with the attached patch?

If you have the courage, it would be awesome if you could also try the
patch without the ‘error/again’ bits.  It’s possible that they aren’t
needed now.  I double-checked and the GnuTLS Guile bindings already
handle GNUTLS_E_AGAIN and GNUTLS_E_INTERRUPTED, so my guess is that this
was just a side effect of dealing with stale TLS sessions:

  https://gitlab.com/gnutls/gnutls/-/blob/master/guile/src/core.c#L1042

Thanks a lot for your help!

Ludo’.

diff --git a/guix/http-client.scm b/guix/http-client.scm
index a2e11a1b73..b9cf0b1a4b 100644
--- a/guix/http-client.scm
+++ b/guix/http-client.scm
@@ -38,7 +38,7 @@
   #:use-module (guix utils)
   #:use-module (guix base64)
   #:autoload   (gcrypt hash) (sha256)
-  #:autoload   (gnutls) (error/invalid-session)
+  #:autoload   (gnutls) (error/invalid-session error/again)
   #:use-module ((guix build utils)
 #:select (mkdir-p dump-port))
   #:use-module ((guix build download)
@@ -163,7 +163,8 @@ reusing stale cached connections."
   (if (or (and (eq? key 'system-error)
(= EPIPE (system-error-errno `(,key ,@args
   (and (eq? key 'gnutls-error)
-   (eq? (first args) error/invalid-session))
+   (memq (first args)
+ (list error/again error/invalid-session)))
   (memq key
 '(bad-response bad-header bad-header-component)))
   #f
@@ -207,15 +208,14 @@ returning."
 ;; Inherit the HTTP proxying property from P.
 (set-http-proxy-port?! buffer (http-proxy-port? p))
 
-(unless (false-if-networking-error
- (begin
-   (for-each (cut write-request <> buffer) batch)
-   (put-bytevector p (get))
-   (force-output p)
-   #t))
-   

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

[Canan Talayhan]

It seems after testing lots of pages this one escaped me since I only
tested the working case.

Please find the quick fix in the link below.
https://pastebin.ubuntu.com/p/s7tWyPHZ8F/

I'm looking forward to making another contribution. Could you please
review it as soon as possible?

Thanks,
Canan Talayhan


On Thu, Apr 22, 2021 at 10:47 PM Christopher Baines  wrote:
>
>
> 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"))
> 

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

[Xinglu Chen]

Hi,

On Fri, Apr 23 2021, Ludovic Courtès wrote:

> 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.

I think that would be a good idea, maybe it could also make having a
default value be optional, like this:

#+begin_src scheme
(define-configuration foo
  (bar (integer) "doc" no-serializer) ;no default
  (baz (string "default") "doc"))
#+end_src

Maxim and I had a discussion about this[1].

[1]: https://yhetil.org/guix-devel/87k0ov7w72@yoctocell.xyz

Re: Best practices for writing services

[Xinglu Chen]

Hi,

On Fri, Apr 23 2021, Maxim Cournoyer wrote:

>> But the problem here is that it doesn’t force the user to configure the
>> field.  In a Git config for example, the user should be forced to set
>> ‘user.name’ and ‘user.email’, otherwise they can’t commit anything.  You
>> will just have to set the default value to ‘disabled’, like this:
>>
>> #+begin_src scheme
>> (define (serialize-string field-name val) ...)
>> (define-maybe string)
>> (define-configuration test-config
>>   (config
>> (maybe-string ’disabled))
>> "docs"")
>> #+end_src
>
> Ah, thanks for explaining, now I understand your point well.
>
> I've just tried something:
>
> --8<---cut here---start->8---
> (define-configuration test-config
>(name (string #f) "Your name"))
> scheme@(guile-user)> (test-config)
> ice-9/boot-9.scm:1669:16: In procedure raise-exception:
> ERROR:
>   1. : "Invalid value for field name: #f"
>   2. 
> --8<---cut here---end--->8---
>
> So you could choose an invalid default value, which would force the user
> to specify it (else they'd get the not so obvious error message above).
> It should be improved too!  I'll see if I can do something.

That would be a workaround for now.  Thank you!