GNU bug report logs - #78233
[PATCH 0/2 electronics-team] Upgrade nextpnr.

Previous Next

Full log

Message #26 received at 78233 <at> debbugs.gnu.org (full text, mbox):

From: Cayetano Santos <csantosb <at> inventati.org>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 78233 <at> debbugs.gnu.org, Gabriel Wicki <gabriel <at> erlikon.ch>,
 Ekaitz Zarraga <ekaitz <at> elenq.tech>
Subject: Re: [bug#78233] [PATCH 2/2] gnu: nextpnr-ice40: Update to 0.8.
Date: Fri, 09 May 2025 20:51:36 +0200

n<#secure method=pgpmime mode=sign>

Hi Maxim,

Thanks for all your comments !  They’re (almost) included in v2 series.

>jeu. 08 mai 2025 at 22:07, Maxim Cournoyer <maxim.cournoyer <at> gmail.com> wrote:

> Interesting, I had never seen the opendir/closedir approach, perhaps
> because the code base in Guix embraces functional rather than imperative
> style.  It'd be more conventional (in Guix at least) to use (ice-9
> ftw)'s scandir to list the files and apply for-each to its value, and I
> think it'd be more succinct too.
>
> Something like:
>
> (with-directory-excursion "icestorm/examples"
>                        (for-each (cut invoke "make" "-C" <>)
>                                  (scandir "."
>                                           (negate (cut member <> '("." ".."))))))

I just learned about cut existence, thanks again; I understand its utility
by looking at examples all around the code. No doubt, this is the correct
alternative.

Unfortunately, I’m unable at this point to use it, I miss the necessary
skills: I fall in frequent "unbound variable <>" errors that I don’t
known yet how to handle, I suspect gexps add a extra layer of complexity
which becomes a barrier to me, sorry for that.

> Hm, this relocation in the same diff as the changes muddled the scope.
> I thought it was a new entry above, so reviewed it as such.  Could you
> please split the relocation as a prior commit with your actual changes
> to review in a 2nd commit on top?  This should give a nicer view, as
> Gabriel also noted.

Let’s forget the relocation by now, as it complicates things with no
real utility by itself. We will sort things out later on.

Just consider that, in reality, nextpnr replaces former nextpnr-ice40
(it is strongly based on it); new nextpnr-ice40 is a new package (even
if it carries the name of previous one).  This is not evident with two commits.

Two commits, without breaking things up, are necessarily somehow
confusing (and kind of useless, to that matter). To me, the only means
of achieving a nicer diff view is using a single commit, where one may
compare new nextpnr with old nextpnr-ice40, at the same time as it becomes
evident that the new nextpnr-ice40 is an addition.

V2 is in its way.

C.

Previous Next

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