| Message ID | 87wnjub5y6.fsf@yoctocell.xyz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#52774] import: elpa: Also check NonGNU ELPA for updates. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
Am Freitag, dem 24.12.2021 um 12:25 +0100 schrieb Xinglu Chen: > * elpa.scm (latest-release): Determine the repository based on the > URL of the > source. > (package-from-gnu.org?): Rename to ... > (package-from-gnu.org-or-nongnu.org?): ...this. > (%elpa-updater): Adjust accordingly. > --- > guix/import/elpa.scm | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm > index edabb88b7a..038379e01b 100644 > --- a/guix/import/elpa.scm > +++ b/guix/import/elpa.scm > @@ -424,7 +424,11 @@ (define (guix-package->elpa-name package) > (define (latest-release package) > "Return an <upstream-release> for the latest release of PACKAGE." > (define name (guix-package->elpa-name package)) > - (define repo 'gnu) > + (define repo > + (let ((url (origin-uri (package-source package)))) > + (if (string-prefix? "https://elpa.nongnu.org" url) > + 'nongnu > + 'gnu))) > > (match (elpa-package-info name repo) > (#f > @@ -443,11 +447,12 @@ (define repo 'gnu) > (urls (list url)) > (signature-urls (list (string-append url ".sig")))))))) > > -(define package-from-gnu.org? > +(define package-from-gnu.org-or-nongnu.org? > (url-predicate (lambda (url) > (let ((uri (string->uri url))) > (and uri > - (string=? (uri-host uri) > "elpa.gnu.org")))))) > + (or (string=? (uri-host uri) > "elpa.gnu.org") > + (string=? (uri-host uri) > "elpa.nongnu.org"))))))) > > (define %elpa-updater > ;; The ELPA updater. We restrict it to packages hosted on > elpa.gnu.org > @@ -455,7 +460,7 @@ (define %elpa-updater > (upstream-updater > (name 'elpa) > (description "Updater for ELPA packages") > - (pred package-from-gnu.org?) > + (pred package-from-gnu.org-or-nongnu.org?) > (latest latest-release))) > > (define elpa-guix-name (cut guix-name "emacs-" <>)) > > base-commit: e8c1562599c7ebee8b7b228237fb0d75d4472a61 Could we make it so that package-from-gnu.org? becomes elpa-package? s.t. (member (elpa-package? pkg) '(gnu nongnu #f)) and its evaluation can be reused? If not, I'd still use a cond or match in the definition of repo. Otherwise LGTM, cheers!
Liliana schrieb am Freitag der 24. Dezember 2021 um 13:17 +01: > Am Freitag, dem 24.12.2021 um 12:25 +0100 schrieb Xinglu Chen: >> * elpa.scm (latest-release): Determine the repository based on the >> URL of the >> source. >> (package-from-gnu.org?): Rename to ... >> (package-from-gnu.org-or-nongnu.org?): ...this. >> (%elpa-updater): Adjust accordingly. >> --- >> guix/import/elpa.scm | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm >> index edabb88b7a..038379e01b 100644 >> --- a/guix/import/elpa.scm >> +++ b/guix/import/elpa.scm >> @@ -424,7 +424,11 @@ (define (guix-package->elpa-name package) >> (define (latest-release package) >> "Return an <upstream-release> for the latest release of PACKAGE." >> (define name (guix-package->elpa-name package)) >> - (define repo 'gnu) >> + (define repo >> + (let ((url (origin-uri (package-source package)))) >> + (if (string-prefix? "https://elpa.nongnu.org" url) >> + 'nongnu >> + 'gnu))) >> >> (match (elpa-package-info name repo) >> (#f >> @@ -443,11 +447,12 @@ (define repo 'gnu) >> (urls (list url)) >> (signature-urls (list (string-append url ".sig")))))))) >> >> -(define package-from-gnu.org? >> +(define package-from-gnu.org-or-nongnu.org? >> (url-predicate (lambda (url) >> (let ((uri (string->uri url))) >> (and uri >> - (string=? (uri-host uri) >> "elpa.gnu.org")))))) >> + (or (string=? (uri-host uri) >> "elpa.gnu.org") >> + (string=? (uri-host uri) >> "elpa.nongnu.org"))))))) >> >> (define %elpa-updater >> ;; The ELPA updater. We restrict it to packages hosted on >> elpa.gnu.org >> @@ -455,7 +460,7 @@ (define %elpa-updater >> (upstream-updater >> (name 'elpa) >> (description "Updater for ELPA packages") >> - (pred package-from-gnu.org?) >> + (pred package-from-gnu.org-or-nongnu.org?) >> (latest latest-release))) >> >> (define elpa-guix-name (cut guix-name "emacs-" <>)) >> >> base-commit: e8c1562599c7ebee8b7b228237fb0d75d4472a61 > Could we make it so that package-from-gnu.org? becomes elpa-package? > s.t. (member (elpa-package? pkg) '(gnu nongnu #f)) and its evaluation > can be reused? Wouldn’t it be enough with just '(gnu nongnu)? We could make the ‘elpa-package?’ procedure memoized to avoid evaluating the same thing twice. > Otherwise LGTM, cheers! Cool, thanks for taking a look!
Am Samstag, dem 25.12.2021 um 10:44 +0100 schrieb Xinglu Chen: > Liliana schrieb am Freitag der 24. Dezember 2021 um 13:17 +01: > > > Am Freitag, dem 24.12.2021 um 12:25 +0100 schrieb Xinglu Chen: > > > * elpa.scm (latest-release): Determine the repository based on > > > the URL of the source. > > > (package-from-gnu.org?): Rename to ... > > > (package-from-gnu.org-or-nongnu.org?): ...this. > > > (%elpa-updater): Adjust accordingly. > > > --- > > > guix/import/elpa.scm | 13 +++++++++---- > > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > > > diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm > > > index edabb88b7a..038379e01b 100644 > > > --- a/guix/import/elpa.scm > > > +++ b/guix/import/elpa.scm > > > @@ -424,7 +424,11 @@ (define (guix-package->elpa-name package) > > > (define (latest-release package) > > > "Return an <upstream-release> for the latest release of > > > PACKAGE." > > > (define name (guix-package->elpa-name package)) > > > - (define repo 'gnu) > > > + (define repo > > > + (let ((url (origin-uri (package-source package)))) > > > + (if (string-prefix? "https://elpa.nongnu.org" url) > > > + 'nongnu > > > + 'gnu))) > > > > > > (match (elpa-package-info name repo) > > > (#f > > > @@ -443,11 +447,12 @@ (define repo 'gnu) > > > (urls (list url)) > > > (signature-urls (list (string-append url ".sig")))))))) > > > > > > -(define package-from-gnu.org? > > > +(define package-from-gnu.org-or-nongnu.org? > > > (url-predicate (lambda (url) > > > (let ((uri (string->uri url))) > > > (and uri > > > - (string=? (uri-host uri) > > > "elpa.gnu.org")))))) > > > + (or (string=? (uri-host uri) > > > "elpa.gnu.org") > > > + (string=? (uri-host uri) > > > "elpa.nongnu.org"))))))) > > > > > > (define %elpa-updater > > > ;; The ELPA updater. We restrict it to packages hosted on > > > elpa.gnu.org > > > @@ -455,7 +460,7 @@ (define %elpa-updater > > > (upstream-updater > > > (name 'elpa) > > > (description "Updater for ELPA packages") > > > - (pred package-from-gnu.org?) > > > + (pred package-from-gnu.org-or-nongnu.org?) > > > (latest latest-release))) > > > > > > (define elpa-guix-name (cut guix-name "emacs-" <>)) > > > > > > base-commit: e8c1562599c7ebee8b7b228237fb0d75d4472a61 > > Could we make it so that package-from-gnu.org? becomes elpa- > > package? s.t. (member (elpa-package? pkg) '(gnu nongnu #f)) and its > > evaluation can be reused? > > Wouldn’t it be enough with just '(gnu nongnu)? > > We could make the ‘elpa-package?’ procedure memoized to avoid > evaluating the same thing twice. The reason for #f is so that packages that are neither from GNU nor NonGNU ELPA are still excluded from the updater. Cheers
Liliana schrieb am Samstag der 25. Dezember 2021 um 11:15 +01: > Am Samstag, dem 25.12.2021 um 10:44 +0100 schrieb Xinglu Chen: >> Liliana schrieb am Freitag der 24. Dezember 2021 um 13:17 +01: >> >> > Am Freitag, dem 24.12.2021 um 12:25 +0100 schrieb Xinglu Chen: >> > > * elpa.scm (latest-release): Determine the repository based on >> > > the URL of the source. >> > > (package-from-gnu.org?): Rename to ... >> > > (package-from-gnu.org-or-nongnu.org?): ...this. >> > > (%elpa-updater): Adjust accordingly. >> > > --- >> > > guix/import/elpa.scm | 13 +++++++++---- >> > > 1 file changed, 9 insertions(+), 4 deletions(-) >> > > >> > > diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm >> > > index edabb88b7a..038379e01b 100644 >> > > --- a/guix/import/elpa.scm >> > > +++ b/guix/import/elpa.scm >> > > @@ -424,7 +424,11 @@ (define (guix-package->elpa-name package) >> > > (define (latest-release package) >> > > "Return an <upstream-release> for the latest release of >> > > PACKAGE." >> > > (define name (guix-package->elpa-name package)) >> > > - (define repo 'gnu) >> > > + (define repo >> > > + (let ((url (origin-uri (package-source package)))) >> > > + (if (string-prefix? "https://elpa.nongnu.org" url) >> > > + 'nongnu >> > > + 'gnu))) >> > > >> > > (match (elpa-package-info name repo) >> > > (#f >> > > @@ -443,11 +447,12 @@ (define repo 'gnu) >> > > (urls (list url)) >> > > (signature-urls (list (string-append url ".sig")))))))) >> > > >> > > -(define package-from-gnu.org? >> > > +(define package-from-gnu.org-or-nongnu.org? >> > > (url-predicate (lambda (url) >> > > (let ((uri (string->uri url))) >> > > (and uri >> > > - (string=? (uri-host uri) >> > > "elpa.gnu.org")))))) >> > > + (or (string=? (uri-host uri) >> > > "elpa.gnu.org") >> > > + (string=? (uri-host uri) >> > > "elpa.nongnu.org"))))))) >> > > >> > > (define %elpa-updater >> > > ;; The ELPA updater. We restrict it to packages hosted on >> > > elpa.gnu.org >> > > @@ -455,7 +460,7 @@ (define %elpa-updater >> > > (upstream-updater >> > > (name 'elpa) >> > > (description "Updater for ELPA packages") >> > > - (pred package-from-gnu.org?) >> > > + (pred package-from-gnu.org-or-nongnu.org?) >> > > (latest latest-release))) >> > > >> > > (define elpa-guix-name (cut guix-name "emacs-" <>)) >> > > >> > > base-commit: e8c1562599c7ebee8b7b228237fb0d75d4472a61 >> > Could we make it so that package-from-gnu.org? becomes elpa- >> > package? s.t. (member (elpa-package? pkg) '(gnu nongnu #f)) and its >> > evaluation can be reused? >> >> Wouldn’t it be enough with just '(gnu nongnu)? >> >> We could make the ‘elpa-package?’ procedure memoized to avoid >> evaluating the same thing twice. > The reason for #f is so that packages that are neither from GNU nor > NonGNU ELPA are still excluded from the updater. I am not sure I follow. If ‘elpa-package?’ returned #f, the following would return #f. (member (elpa-package? pkg) '(gnu nongnu)) Meaning that PKG is not hosted on GNU or NonGNU ELPA, and thus, should not be checked for updates. When including #f in the list, the value returned by the ‘member’ expression would be '(#f), which means that PKG would be checked for updates, even though it shouldn’t.
Am Samstag, dem 25.12.2021 um 15:31 +0100 schrieb Xinglu Chen: > Liliana schrieb am Samstag der 25. Dezember 2021 um 11:15 +01: > > > Am Samstag, dem 25.12.2021 um 10:44 +0100 schrieb Xinglu Chen: > > > Liliana schrieb am Freitag der 24. Dezember 2021 um 13:17 +01: > > > > > > > Am Freitag, dem 24.12.2021 um 12:25 +0100 schrieb Xinglu Chen: > > > > > * elpa.scm (latest-release): Determine the repository based > > > > > on > > > > > the URL of the source. > > > > > (package-from-gnu.org?): Rename to ... > > > > > (package-from-gnu.org-or-nongnu.org?): ...this. > > > > > (%elpa-updater): Adjust accordingly. > > > > > --- > > > > > guix/import/elpa.scm | 13 +++++++++---- > > > > > 1 file changed, 9 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm > > > > > index edabb88b7a..038379e01b 100644 > > > > > --- a/guix/import/elpa.scm > > > > > +++ b/guix/import/elpa.scm > > > > > @@ -424,7 +424,11 @@ (define (guix-package->elpa-name > > > > > package) > > > > > (define (latest-release package) > > > > > "Return an <upstream-release> for the latest release of > > > > > PACKAGE." > > > > > (define name (guix-package->elpa-name package)) > > > > > - (define repo 'gnu) > > > > > + (define repo > > > > > + (let ((url (origin-uri (package-source package)))) > > > > > + (if (string-prefix? "https://elpa.nongnu.org" url) > > > > > + 'nongnu > > > > > + 'gnu))) > > > > > > > > > > (match (elpa-package-info name repo) > > > > > (#f > > > > > @@ -443,11 +447,12 @@ (define repo 'gnu) > > > > > (urls (list url)) > > > > > (signature-urls (list (string-append url > > > > > ".sig")))))))) > > > > > > > > > > -(define package-from-gnu.org? > > > > > +(define package-from-gnu.org-or-nongnu.org? > > > > > (url-predicate (lambda (url) > > > > > (let ((uri (string->uri url))) > > > > > (and uri > > > > > - (string=? (uri-host uri) > > > > > "elpa.gnu.org")))))) > > > > > + (or (string=? (uri-host uri) > > > > > "elpa.gnu.org") > > > > > + (string=? (uri-host uri) > > > > > "elpa.nongnu.org"))))))) > > > > > > > > > > (define %elpa-updater > > > > > ;; The ELPA updater. We restrict it to packages hosted on > > > > > elpa.gnu.org > > > > > @@ -455,7 +460,7 @@ (define %elpa-updater > > > > > (upstream-updater > > > > > (name 'elpa) > > > > > (description "Updater for ELPA packages") > > > > > - (pred package-from-gnu.org?) > > > > > + (pred package-from-gnu.org-or-nongnu.org?) > > > > > (latest latest-release))) > > > > > > > > > > (define elpa-guix-name (cut guix-name "emacs-" <>)) > > > > > > > > > > base-commit: e8c1562599c7ebee8b7b228237fb0d75d4472a61 > > > > Could we make it so that package-from-gnu.org? becomes elpa- > > > > package? s.t. (member (elpa-package? pkg) '(gnu nongnu #f)) and > > > > its > > > > evaluation can be reused? > > > > > > Wouldn’t it be enough with just '(gnu nongnu)? > > > > > > We could make the ‘elpa-package?’ procedure memoized to avoid > > > evaluating the same thing twice. > > The reason for #f is so that packages that are neither from GNU nor > > NonGNU ELPA are still excluded from the updater. > > I am not sure I follow. If ‘elpa-package?’ returned #f, the > following would return #f. > > (member (elpa-package? pkg) '(gnu nongnu)) > > Meaning that PKG is not hosted on GNU or NonGNU ELPA, and thus, > should not be checked for updates. > > When including #f in the list, the value returned by the ‘member’ > expression would be '(#f), which means that PKG would be checked for > updates, even though it shouldn’t. I think you're misunderstanding what I'm saying, but that might be because I worded it badly. My suggestion was to make elpa-package? a "predicate with meaning", which returns #f if PKG is not an ELPA package, and a truthy value otherwise, said truthy value being the symbol 'gnu or 'nongnu at the moment (which could be expanded to e.g. include 'melpa or 'some-other-law-abiding-elpa in case the ELPA updater is found to work for those as well). Saying that it satisfies the above member function was a shorthand for that.
diff --git a/guix/import/elpa.scm b/guix/import/elpa.scm index edabb88b7a..038379e01b 100644 --- a/guix/import/elpa.scm +++ b/guix/import/elpa.scm @@ -424,7 +424,11 @@ (define (guix-package->elpa-name package) (define (latest-release package) "Return an <upstream-release> for the latest release of PACKAGE." (define name (guix-package->elpa-name package)) - (define repo 'gnu) + (define repo + (let ((url (origin-uri (package-source package)))) + (if (string-prefix? "https://elpa.nongnu.org" url) + 'nongnu + 'gnu))) (match (elpa-package-info name repo) (#f @@ -443,11 +447,12 @@ (define repo 'gnu) (urls (list url)) (signature-urls (list (string-append url ".sig")))))))) -(define package-from-gnu.org? +(define package-from-gnu.org-or-nongnu.org? (url-predicate (lambda (url) (let ((uri (string->uri url))) (and uri - (string=? (uri-host uri) "elpa.gnu.org")))))) + (or (string=? (uri-host uri) "elpa.gnu.org") + (string=? (uri-host uri) "elpa.nongnu.org"))))))) (define %elpa-updater ;; The ELPA updater. We restrict it to packages hosted on elpa.gnu.org @@ -455,7 +460,7 @@ (define %elpa-updater (upstream-updater (name 'elpa) (description "Updater for ELPA packages") - (pred package-from-gnu.org?) + (pred package-from-gnu.org-or-nongnu.org?) (latest latest-release))) (define elpa-guix-name (cut guix-name "emacs-" <>))