GNU bug report logs -
#49522
[PATCH] weather: Don't look for exported package replacements twice.
Previous Next
Reported by: Christopher Baines <mail <at> cbaines.net>
Date: Sun, 11 Jul 2021 11:57:01 UTC
Severity: normal
Tags: patch
Done: Christopher Baines <mail <at> cbaines.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 49522 in the body.
You can then email your comments to 49522 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
guix-patches <at> gnu.org
:
bug#49522
; Package
guix-patches
.
(Sun, 11 Jul 2021 11:57:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Christopher Baines <mail <at> cbaines.net>
:
New bug report received and forwarded. Copy sent to
guix-patches <at> gnu.org
.
(Sun, 11 Jul 2021 11:57:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
There could be performance issues with member here, but only if there are lots
of replacements.
* guix/scripts/weather.scm (all-packages): Return all exported packages, plus
non exported replacements, rather than including exported replacements twice.
---
guix/scripts/weather.scm | 35 ++++++++++++++++++++++++-----------
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 06312d65a2..0f70dc8460 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -53,17 +53,30 @@
#:export (guix-weather))
(define (all-packages)
- "Return the list of public packages we are going to query."
- (fold-packages (lambda (package result)
- (match (package-replacement package)
- ((? package? replacement)
- (cons* replacement package result))
- (#f
- (cons package result))))
- '()
-
- ;; Dismiss deprecated packages but keep hidden packages.
- #:select? (negate package-superseded)))
+ "Return the list of packages we are going to query."
+ (let* ((packages
+ (fold-packages (lambda (package result)
+ (cons package result))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded)))
+ (non-exported-replacement-packages
+ (fold-packages (lambda (package result)
+ (let ((replacement (package-replacement package)))
+ (if (and replacement
+ ;; Avoid double couting replacements
+ ;; that are themselves exported
+ (not (member replacement packages)))
+ (cons replacement result)
+ result)))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded))))
+ (append
+ packages
+ non-exported-replacement-packages)))
(define (call-with-progress-reporter reporter proc)
"This is a variant of 'call-with-progress-reporter' that works with monadic
--
2.32.0
Information forwarded
to
guix-patches <at> gnu.org
:
bug#49522
; Package
guix-patches
.
(Mon, 19 Jul 2021 17:12:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 49522 <at> debbugs.gnu.org (full text, mbox):
Hi!
Christopher Baines <mail <at> cbaines.net> skribis:
> There could be performance issues with member here, but only if there are lots
> of replacements.
>
> * guix/scripts/weather.scm (all-packages): Return all exported packages, plus
> non exported replacements, rather than including exported replacements twice.
[...]
> + "Return the list of packages we are going to query."
> + (let* ((packages
> + (fold-packages (lambda (package result)
> + (cons package result))
> + '()
> +
> + ;; Dismiss deprecated packages but keep hidden packages.
> + #:select? (negate package-superseded)))
> + (non-exported-replacement-packages
> + (fold-packages (lambda (package result)
> + (let ((replacement (package-replacement package)))
> + (if (and replacement
> + ;; Avoid double couting replacements
> + ;; that are themselves exported
> + (not (member replacement packages)))
> + (cons replacement result)
> + result)))
> + '()
> +
> + ;; Dismiss deprecated packages but keep hidden packages.
> + #:select? (negate package-superseded))))
> + (append
> + packages
> + non-exported-replacement-packages)))
Is the goal to delete duplicates?
In that case, how about wrapping the existing expression in
(delete-duplicates … eq?) ?
(Note that (member x lst) is O(N) is the number of packages, so the code
above is quadratic in the number of packages, which should be avoided.
Also, packages cannot be meaningfully compared with ‘equal?’ (which is
expensive in case of different packages), so ‘eq?’ should be preferred.)
Thanks,
Ludo’.
Information forwarded
to
guix-patches <at> gnu.org
:
bug#49522
; Package
guix-patches
.
(Sun, 01 Aug 2021 15:02:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 49522 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> writes:
> Hi!
>
> Christopher Baines <mail <at> cbaines.net> skribis:
>
>> There could be performance issues with member here, but only if there are lots
>> of replacements.
>>
>> * guix/scripts/weather.scm (all-packages): Return all exported packages, plus
>> non exported replacements, rather than including exported replacements twice.
>
> [...]
>
>> + "Return the list of packages we are going to query."
>> + (let* ((packages
>> + (fold-packages (lambda (package result)
>> + (cons package result))
>> + '()
>> +
>> + ;; Dismiss deprecated packages but keep hidden packages.
>> + #:select? (negate package-superseded)))
>> + (non-exported-replacement-packages
>> + (fold-packages (lambda (package result)
>> + (let ((replacement (package-replacement package)))
>> + (if (and replacement
>> + ;; Avoid double couting replacements
>> + ;; that are themselves exported
>> + (not (member replacement packages)))
>> + (cons replacement result)
>> + result)))
>> + '()
>> +
>> + ;; Dismiss deprecated packages but keep hidden packages.
>> + #:select? (negate package-superseded))))
>> + (append
>> + packages
>> + non-exported-replacement-packages)))
>
> Is the goal to delete duplicates?
Not really, the goal is to have a list with no duplicates.
> In that case, how about wrapping the existing expression in
> (delete-duplicates … eq?) ?
>
> (Note that (member x lst) is O(N) is the number of packages, so the code
> above is quadratic in the number of packages, which should be avoided.
> Also, packages cannot be meaningfully compared with ‘equal?’ (which is
> expensive in case of different packages), so ‘eq?’ should be preferred.)
Replacing member with memq seems sensible.
My understanding of delete duplicates is that it's O(N^2), because it
constructs a new list, and keeps searching it in linear time. While that
sounds better, doing a linear search of packages for each replacement
should be quicker, assuming there's only a few replacements compared to
the number of packages.
I'm fine with either though, I don't actually know which is faster, and
it probably doesn't matter anyway. I'll send a patch with a
delete-duplicates approach.
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
guix-patches <at> gnu.org
:
bug#49522
; Package
guix-patches
.
(Sun, 01 Aug 2021 15:46:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 49522 <at> debbugs.gnu.org (full text, mbox):
* guix/scripts/weather.scm (all-packages): Delete duplicates, so that exported
replacements aren't included twice.
---
guix/scripts/weather.scm | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/guix/scripts/weather.scm b/guix/scripts/weather.scm
index 06312d65a2..60a697d1ac 100644
--- a/guix/scripts/weather.scm
+++ b/guix/scripts/weather.scm
@@ -54,16 +54,18 @@
(define (all-packages)
"Return the list of public packages we are going to query."
- (fold-packages (lambda (package result)
- (match (package-replacement package)
- ((? package? replacement)
- (cons* replacement package result))
- (#f
- (cons package result))))
- '()
-
- ;; Dismiss deprecated packages but keep hidden packages.
- #:select? (negate package-superseded)))
+ (delete-duplicates
+ (fold-packages (lambda (package result)
+ (match (package-replacement package)
+ ((? package? replacement)
+ (cons* replacement package result))
+ (#f
+ (cons package result))))
+ '()
+
+ ;; Dismiss deprecated packages but keep hidden packages.
+ #:select? (negate package-superseded))
+ eq?))
(define (call-with-progress-reporter reporter proc)
"This is a variant of 'call-with-progress-reporter' that works with monadic
--
2.32.0
Information forwarded
to
guix-patches <at> gnu.org
:
bug#49522
; Package
guix-patches
.
(Wed, 01 Sep 2021 21:31:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 49522 <at> debbugs.gnu.org (full text, mbox):
Hi,
Christopher Baines <mail <at> cbaines.net> skribis:
> * guix/scripts/weather.scm (all-packages): Delete duplicates, so that exported
> replacements aren't included twice.
LGTM, and apologies for the delay!
Ludo’.
Reply sent
to
Christopher Baines <mail <at> cbaines.net>
:
You have taken responsibility.
(Fri, 03 Sep 2021 09:26:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Christopher Baines <mail <at> cbaines.net>
:
bug acknowledged by developer.
(Fri, 03 Sep 2021 09:26:01 GMT)
Full text and
rfc822 format available.
Message #22 received at 49522-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Ludovic Courtès <ludo <at> gnu.org> writes:
> Hi,
>
> Christopher Baines <mail <at> cbaines.net> skribis:
>
>> * guix/scripts/weather.scm (all-packages): Delete duplicates, so that exported
>> replacements aren't included twice.
>
> LGTM, and apologies for the delay!
Great, I've pushed this now as 9540323458de87b0b8aa421e449a4fe27af7c393.
[signature.asc (application/pgp-signature, inline)]
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Fri, 01 Oct 2021 11:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 200 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.