guix import pypi foo@1.2.3 breaks

  • Done
  • quality assurance status badge
Details
3 participants
  • Maxim Cournoyer
  • Lulu
  • zimoun
Owner
unassigned
Submitted by
zimoun
Severity
normal
Z
Z
zimoun wrote on 16 Oct 2020 15:48
(address . bug-guix@gnu.org)
86wnzqz3h8.fsf@gmail.com
Dear,

The syntax across the different importers are not uniform. Especially,
compare:

guix import hackage mtl@2.1.3.1
guix import pypi itsdangerous@1.1.0

and worse, the option ’--recursive’ leads to an error for the latter.
Note that instead:

guix import pypi itsdangerous/1.1.0

perfectly works, even the option ’--recursive’.

All the information is there, and the UI has to be fixed. It is a
perfect first contribution fixes.


All the best,
simon

PS:
Reported by Zelphir Kaltstahl.
Z
Z
zimoun wrote on 16 Oct 2020 15:51
tags 44030 easy
(address . control@debbugs.gnu.org)
86pn5iz3c0.fsf@gmail.com
tags 44030 easy
thanks
L
[bug#44030] [PATCH] guix: import: Add versioning syntax to pypi importer.
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
458803687.67759.1603652981842@office.mailbox.org
Use @ as the unified versioning syntax, as per crate and
hackage importers, plus minor spacing fixes.


Toggle diff (32 lines)
diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 15116e349d..add2a7ed7a 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -118,13 +118,15 @@
(define (pypi-fetch name)
"Return a <pypi-project> record for package NAME, or #f on failure."
- (and=> (json-fetch (string-append "https://pypi.org/pypi/" name "/json"))
- json->pypi-project))
+ ;; Convert @ in package name to / to access the correct URL.
+ (let ((versioned-name (string-join (string-split name #\@) "/")))
+ (and=> (json-fetch (string-append "https://pypi.org/pypi/" name "/json"))
+ json->pypi-project)))
;; For packages found on PyPI that lack a source distribution.
(define-condition-type &missing-source-error &error
missing-source-error?
- (package missing-source-error-package))
+ (package missing-source-error-package))
(define (latest-source-release pypi-package)
"Return the latest source release for PYPI-PACKAGE."
@@ -371,7 +373,7 @@ be extracted in a temporary directory."
(invoke "tar" "xf" archive "-C" dir)))
(let ((requires.txt-files
(find-files dir (lambda (abs-file-name _)
- (string-match "\\.egg-info/requires.txt$"
+ (string-match "\\.egg-info/requires.txt$"
abs-file-name)))))
(match requires.txt-files
(()
L
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
1085530218.67944.1603656272351@office.mailbox.org
Ah, heck, I used an old revision of the file when I generated the diff.
Sorry about that. Here's the properly working patch:


Toggle diff (32 lines)
diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 15116e349d..1ec1ecbfa1 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -118,13 +118,15 @@
(define (pypi-fetch name)
"Return a <pypi-project> record for package NAME, or #f on failure."
- (and=> (json-fetch (string-append "https://pypi.org/pypi/" name "/json"))
- json->pypi-project))
+ ;; Convert @ in package name to / to access the correct URL.
+ (let ((versioned-name (string-join (string-split name #\@) "/")))
+ (and=> (json-fetch (string-append "https://pypi.org/pypi/" versioned-name "/json"))
+ json->pypi-project)))
;; For packages found on PyPI that lack a source distribution.
(define-condition-type &missing-source-error &error
missing-source-error?
- (package missing-source-error-package))
+ (package missing-source-error-package))
(define (latest-source-release pypi-package)
"Return the latest source release for PYPI-PACKAGE."
@@ -371,7 +373,7 @@ be extracted in a temporary directory."
(invoke "tar" "xf" archive "-C" dir)))
(let ((requires.txt-files
(find-files dir (lambda (abs-file-name _)
- (string-match "\\.egg-info/requires.txt$"
+ (string-match "\\.egg-info/requires.txt$"
abs-file-name)))))
(match requires.txt-files
(()
L
guix import pypi foo@1.2.3 breaks
(name . 44030@debbugs.gnu.org)(address . 44030@debbugs.gnu.org)
1850369330.68130.1603659032091@office.mailbox.org
I just sent in a patch to fix this for the pypi importer, although
ideally we'd want versioning support for all importers with a uniform
syntax.

I poked and prodded at the RubyGems API to see if it can do what PyPI
can. It's unfortunately much more limited: API v1 provides a method
for querying all versions of a gem [1] and v2 provides a method for
querying specific versions of a gem [2], although neither of them
provides information about dependency versions, so we can't rely on it
for recursive imports (since it would try to import latest versions of
all dependencies). There's a method that fetches dependency and
versioning info for all versions of all given gems but (probably due
to the verbosity of JSON/YAML) it's in binary Ruby serialisation
format [3]. I can try to hack a parser that surgically extracts
dependency info from a given gem version. What do you think?

ELPA runs into a similar problem in that it provides tarballs/files
for older versions but dependency info is only provided in the repo
file. (MELPA tries to directly peg all packages to their respective
Git repos' trunks.)

I presume it's much simpler with the gnu importer, as it's only a
matter of pointing the FTP fetch in the right direction, although I
couldn't confirm it, as the gnu importer doesn't work for me since my
ISP blocks PGP keyserver ports.

I need to take a closer look at CRAN, CPAN, TeXLive and opam.

--
Lulu

Z
Z
zimoun wrote on 26 Oct 2020 16:49
Re: bug#44030: [PATCH] guix: import: Add versioning syntax to pypi importer.
86y2jt3s3p.fsf@gmail.com
Hi,

Thank you for working on this.

On Sun, 25 Oct 2020 at 23:04, Lulu <me@erkin.party> wrote:
Toggle quote (3 lines)
> Ah, heck, I used an old revision of the file when I generated the diff.
> Sorry about that. Here's the properly working patch:

That’s fine. Usually to ease the reading, it is a good habit to
increase the version of the patch, (see the option ’reroll-count’ of
git-format-patch).


However, the 2 patches are not proper commit, right? You can locally
commit your changes and send them. Basically, it means the same thing
but including the commit message. Here it looks like:

Toggle snippet (8 lines)
import: pypi: Allow ’@’ for specifying the version.

Fixes <https://bugs.gnu.org/44030>.

* guix/import/pypi.json (pypi-fetch): Allow ’@’ for specifying the
version.

Well, I let you find more inspiration in previous commit messages. ;-)


Toggle quote (15 lines)
> diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
> index 15116e349d..1ec1ecbfa1 100644
> --- a/guix/import/pypi.scm
> +++ b/guix/import/pypi.scm
> @@ -118,13 +118,15 @@
>
> (define (pypi-fetch name)
> "Return a <pypi-project> record for package NAME, or #f on failure."
> - (and=> (json-fetch (string-append "https://pypi.org/pypi/" name "/json"))
> - json->pypi-project))
> + ;; Convert @ in package name to / to access the correct URL.
> + (let ((versioned-name (string-join (string-split name #\@) "/")))
> + (and=> (json-fetch (string-append "https://pypi.org/pypi/" versioned-name "/json"))
> + json->pypi-project)))

If you feel adventurous, you could try to add a test in the file
“tests/pypi.scm”. :-)

Toggle quote (6 lines)
> ;; For packages found on PyPI that lack a source distribution.
> (define-condition-type &missing-source-error &error
> missing-source-error?
> - (package missing-source-error-package))
> + (package missing-source-error-package))

Why is this line modified?


Toggle quote (9 lines)
> (define (latest-source-release pypi-package)
> "Return the latest source release for PYPI-PACKAGE."
> @@ -371,7 +373,7 @@ be extracted in a temporary directory."
> (invoke "tar" "xf" archive "-C" dir)))
> (let ((requires.txt-files
> (find-files dir (lambda (abs-file-name _)
> - (string-match "\\.egg-info/requires.txt$"
> + (string-match "\\.egg-info/requires.txt$"

Idem.


All the best,
simon
Z
Z
zimoun wrote on 26 Oct 2020 16:58
Re: bug#44030: guix import pypi foo@1.2.3 breaks
86v9ex3rnh.fsf@gmail.com
Hi,

On Sun, 25 Oct 2020 at 23:50, Lulu <me@erkin.party> wrote:
Toggle quote (4 lines)
> I just sent in a patch to fix this for the pypi importer, although
> ideally we'd want versioning support for all importers with a uniform
> syntax.

The uniform syntax is ’@’. However, it is importer by importer (case by
case) since the external API is different. AFAIU.


Speaking about “guix import pypi”, the “--recursive” options still
ungracefully fails; even with you patch.


Toggle quote (24 lines)
> I poked and prodded at the RubyGems API to see if it can do what PyPI
> can. It's unfortunately much more limited: API v1 provides a method
> for querying all versions of a gem [1] and v2 provides a method for
> querying specific versions of a gem [2], although neither of them
> provides information about dependency versions, so we can't rely on it
> for recursive imports (since it would try to import latest versions of
> all dependencies). There's a method that fetches dependency and
> versioning info for all versions of all given gems but (probably due
> to the verbosity of JSON/YAML) it's in binary Ruby serialisation
> format [3]. I can try to hack a parser that surgically extracts
> dependency info from a given gem version. What do you think?
>
> ELPA runs into a similar problem in that it provides tarballs/files
> for older versions but dependency info is only provided in the repo
> file. (MELPA tries to directly peg all packages to their respective
> Git repos' trunks.)
>
> I presume it's much simpler with the gnu importer, as it's only a
> matter of pointing the FTP fetch in the right direction, although I
> couldn't confirm it, as the gnu importer doesn't work for me since my
> ISP blocks PGP keyserver ports.
>
> I need to take a closer look at CRAN, CPAN, TeXLive and opam.

Hum? You are discussing this bug [1], right? So please could you
discuss overthere.



All the best,
simon
L
[bug#44030] [PATCH] import: pypi: Add '@' syntax for specifying the package version.
(name . guix-patches@gnu.org)(address . guix-patches@gnu.org)
1052540792.123562.1603913548037@office.mailbox.org
* guix/import/pypi.scm (pypi-fetch): Add ’@’ syntax for specifying the package version.
---
guix/import/pypi.scm | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Toggle diff (19 lines)
diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 15116e349d..559be4a98b 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -118,8 +118,10 @@
(define (pypi-fetch name)
"Return a <pypi-project> record for package NAME, or #f on failure."
- (and=> (json-fetch (string-append "https://pypi.org/pypi/" name "/json"))
- json->pypi-project))
+ ;; Convert @ in package name to / to access the correct URL.
+ (let ((versioned-name (string-join (string-split name #\@) "/")))
+ (and=> (json-fetch (string-append "https://pypi.org/pypi/" versioned-name "/json"))
+ json->pypi-project)))
;; For packages found on PyPI that lack a source distribution.
(define-condition-type &missing-source-error &error
--
2.29.1
L
guix import pypi foo@1.2.3 breaks
(name . 44030@debbugs.gnu.org)(address . 44030@debbugs.gnu.org)
1958052003.4017.1604009380285@office.mailbox.org
Okay, I tested this a bit and it seems to work fine. I'm going to write formal
tests for it (and other importers, if possible) later.

--
Lulu
Z
Z
zimoun wrote on 2 Nov 2020 13:15
(name . Lulu)(address . me@erkin.party)(name . 44030@debbugs.gnu.org)(address . 44030@debbugs.gnu.org)
87r1pc54zt.fsf@gmail.com
Hi,

On Fri, 30 Oct 2020 at 01:09, Lulu <me@erkin.party> wrote:
Toggle quote (3 lines)
> Okay, I tested this a bit and it seems to work fine. I'm going to write formal
> tests for it (and other importers, if possible) later.

Usually, we try to use the option ’--reroll-count’ of git-format-patch
for the second (or third or more) version. It helps the review to know
– from the subject – which is the email to read, etc. Well, next
time. :-)


What do you mean by write “formal tests”?


All the best,
simon
M
M
Maxim Cournoyer wrote on 21 Nov 2021 07:14
(name . zimoun)(address . zimon.toutoune@gmail.com)(address . 44030-done@debbugs.gnu.org)
87h7c6c9xy.fsf@gmail.com
Hi,

zimoun <zimon.toutoune@gmail.com> writes:

Toggle quote (22 lines)
> Dear,
>
> The syntax across the different importers are not uniform. Especially,
> compare:
>
> guix import hackage mtl@2.1.3.1
> guix import pypi itsdangerous@1.1.0
>
> and worse, the option ’--recursive’ leads to an error for the latter.
> Note that instead:
>
> guix import pypi itsdangerous/1.1.0
>
> perfectly works, even the option ’--recursive’.
>
> All the information is there, and the UI has to be fixed. It is a
> perfect first contribution fixes.
>
>
> All the best,
> simon

This works now:

$ ./pre-inst-env guix import pypi ansible@4.4.0
Starting download of /tmp/guix-file.mtcWzL
…4.0.tar.gz 33.7MiB 49.2MiB/s 00:01 [##################] 100.0%
(package
(name "python-ansible")
(version "4.4.0")
(source
(origin
(method url-fetch)
(uri (pypi-uri "ansible" version))
(sha256
(base32 "031n22j0lsmh69x6i6gkva81j68b4yzh1pbg3q2h4bknl85q46ag"))))
(build-system python-build-system)
(propagated-inputs (list python-ansible-core))
(synopsis "Radically simple IT automation")
(description "Radically simple IT automation")
(license #f))

$ ./pre-inst-env guix import pypi ansible@4.4.1
guix import: error: no source release for pypi package ansible 4.4.1

Closing!

Maxim
Closed
Z
Z
zimoun wrote on 22 Nov 2021 10:07
85czms1ruq.fsf@gmail.com
Hi Maxim,

On Sun, 21 Nov 2021 at 07:14, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
Toggle quote (10 lines)
> zimoun <zimon.toutoune@gmail.com> writes:

>> The syntax across the different importers are not uniform. Especially,
>> compare:
>>
>> guix import hackage mtl@2.1.3.1
>> guix import pypi itsdangerous@1.1.0
>>
>> and worse, the option ’--recursive’ leads to an error for the latter.

Yes, this is indeed fixed.


Toggle quote (6 lines)
> > Note that instead:
> >
> > guix import pypi itsdangerous/1.1.0
> >
> > perfectly works, even the option ’--recursive’.

Not this one. Well, except if we consider it is a feature. ;-)



Cheers,
simon
?
Your comment

This issue is archived.

To comment on this conversation send an email to 44030@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 44030
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch