Message ID | 87k0cbjf7n.fsf@corp.mail.ru |
---|---|
State | Accepted |
Headers | show |
Series | [bug#54644,1/7] gnu: rust-nom-locate-4: Add 4.0.0 | expand |
Evgenii Lepikhin via Guix-patches via schreef op do 31-03-2022 om 00:35 [+0300]: > + (synopsis "Fast float parsing conversion routines") > + (description "Fast float parsing conversion routines.") The description is missing. Aslo, every rust package seems to aim to be fast, so no need to mention fastness, it borders on marketing phrasing (see (guix)Synopses and Descriptions) Greetings, Maxime.
Evgenii Lepikhin via Guix-patches via schreef op do 31-03-2022 om 00:35 [+0300]: > +(define-public rust-minimal-lexical-0.2 > + (package > + (name "rust-minimal-lexical") > + (version "0.2.1") > + (source > + (origin > + (method url-fetch) > + (uri (crate-uri "minimal-lexical" version)) > + (file-name (string-append name "-" version ".tar.gz")) > + (sha256 > + (base32 "16ppc5g84aijpri4jzv14rvcnslvlpphbszc7zzp6vfkddf4qdb8")))) > + (build-system cargo-build-system) > + (home-page "https://github.com/Alexhuszagh/minimal-lexical") > + (synopsis "Fast float parsing conversion routines") > + (description "Fast float parsing conversion routines.") > + (license (list license:expat license:asl2.0)))) > + > (define-public rust-minimal-lexical-0.1 > (package > (name "rust-minimal-lexical") Instead of defining a new package, WDYT renaming 'rust-minimal-lexical- 0.1' to 'rust-minimal-lexical' and updating it to 0.2? If so, you'll have to check if rust-nom@7 still builds. Greetings, Maxime.
On Thu, Mar 31, 2022 at 02:01:31PM +0200, Maxime Devos wrote: > Evgenii Lepikhin via Guix-patches via schreef op do 31-03-2022 om 00:35 > [+0300]: > > +(define-public rust-minimal-lexical-0.2 > > + (package > > + (name "rust-minimal-lexical") > > + (version "0.2.1") > > + (source > > + (origin > > + (method url-fetch) > > + (uri (crate-uri "minimal-lexical" version)) > > + (file-name (string-append name "-" version ".tar.gz")) > > + (sha256 > > + (base32 "16ppc5g84aijpri4jzv14rvcnslvlpphbszc7zzp6vfkddf4qdb8")))) > > + (build-system cargo-build-system) > > + (home-page "https://github.com/Alexhuszagh/minimal-lexical") > > + (synopsis "Fast float parsing conversion routines") > > + (description "Fast float parsing conversion routines.") > > + (license (list license:expat license:asl2.0)))) > > + > > (define-public rust-minimal-lexical-0.1 > > (package > > (name "rust-minimal-lexical") > > Instead of defining a new package, WDYT renaming 'rust-minimal-lexical- > 0.1' to 'rust-minimal-lexical' and updating it to 0.2? If so, you'll > have to check if rust-nom@7 still builds. semver is very strongly followed in the rust community, so 0.1 isn't necessarily compatible with 0.2. That's how we've ended up with the numerical suffix on all the rust packages.
Efraim Flashner schreef op do 31-03-2022 om 17:47 [+0300]: > On Thu, Mar 31, 2022 at 02:01:31PM +0200, Maxime Devos wrote: > > Evgenii Lepikhin via Guix-patches via schreef op do 31-03-2022 om > 00:35 > > [+0300]: > > > +(define-public rust-minimal-lexical-0.2 > > > + (package > > > + (name "rust-minimal-lexical") > > > + (version "0.2.1") > > > + (source > > > + (origin > > > + (method url-fetch) > > > + (uri (crate-uri "minimal-lexical" version)) > > > + (file-name (string-append name "-" version ".tar.gz")) > > > + (sha256 > > > + (base32 > "16ppc5g84aijpri4jzv14rvcnslvlpphbszc7zzp6vfkddf4qdb8")))) > > > + (build-system cargo-build-system) > > > + (home-page "https://github.com/Alexhuszagh/minimal-lexical") > > > + (synopsis "Fast float parsing conversion routines") > > > + (description "Fast float parsing conversion routines.") > > > + (license (list license:expat license:asl2.0)))) > > > + > > > (define-public rust-minimal-lexical-0.1 > > > (package > > > (name "rust-minimal-lexical") > > > > Instead of defining a new package, WDYT renaming 'rust-minimal- > lexical- > > 0.1' to 'rust-minimal-lexical' and updating it to 0.2? If so, > you'll > > have to check if rust-nom@7 still builds. > > semver is very strongly followed in the rust community, so 0.1 isn't > necessarily compatible with 0.2. That's how we've ended up with the > numerical suffix on all the rust packages. It isn't 100% theoretically compatible. However, it might be _sufficiently_ compatible for all packages using rust-minimal-lexical in Guix. The same seems to hold often for non-Rust packages, I don't see a reason to make an exception for Rust. Why do we package separate 0.Y versions (or separate major versions, for that matter) for Rust packages and not for, say, Guile, Python and C packages? Greetings, Maxime.
Hello there, On 2022-03-31 17:40, Maxime Devos wrote: >> > Instead of defining a new package, WDYT renaming 'rust-minimal- >> lexical- >> > 0.1' to 'rust-minimal-lexical' and updating it to 0.2? If so, >> you'll >> > have to check if rust-nom@7 still builds. >> >> semver is very strongly followed in the rust community, so 0.1 isn't >> necessarily compatible with 0.2. That's how we've ended up with the >> numerical suffix on all the rust packages. > > It isn't 100% theoretically compatible. However, it might be > _sufficiently_ compatible for all packages using rust-minimal-lexical > in Guix. The same seems to hold often for non-Rust packages, I don't > see a reason to make an exception for Rust. Why do we package separate > 0.Y versions (or separate major versions, for that matter) for Rust > packages and not for, say, Guile, Python and C packages? nom-7.0 is not forward compatible with nom-7.1. I need 7.1 for future contributions and this is why I added it into patchset. In developer's manifest of nom-7.1 dependency on rust-minimal-lexical has been updated from 0.1 to 0.2. For both versions dependencies on exact versions on rust-minimal-lexical are pinned by crate developers and we cannot change it at will. Multiple packages depends on rust-nom-7. At least on of them, rust-rusticata-macros, requires nom 7.0 exactly. Thus, we need rust-nom-7.1 as a separate package. g It looks like the standard here in Guix: $ grep -P 'define-public [^ ]+-\d+[.]\d+$' gnu/packages/crates-io.scm | wc -l 2320 Rust packages in form PACKAGENAME-X.Y $ grep -P 'define-public [^ ]+-\d+$' gnu/packages/crates-io.scm | wc -l 536 Rust packages in form PACKAGENAME-X $ grep -P 'define-public [^ ]+\D$' gnu/packages/crates-io.scm|wc -l 3 Rust packages in form PACKAGENAME Thank you for review! I am newbie Guix contributor. --
Evgenii Lepikhin schreef op vr 01-04-2022 om 01:30 [+0300]: > nom-7.0 is not forward compatible with nom-7.1. I need 7.1 for future > contributions and this is why I added it into patchset. Related: to reduce duplication, you can do (define-public rust-nom-7.1 (package (inherit rust-nom-7) [...])) that way, you can ‘copy’ the synopsis of rust-nom-7 without writing it down again. Also, if semver is used here, nom-7.0 is compatible with nom-7.1 (not sure about the terminology here -- forwards? backwards?, but updating it shouldn't cause any problems if semver is followed correctlu). > In developer's manifest of nom-7.1 dependency on rust-minimal-lexical > has been updated from 0.1 to 0.2. For both versions dependencies on > exact versions on rust-minimal-lexical are pinned by crate developers > and we cannot change it at will We can do that, with some 'substitute*'. It is not guaranteed that it will actually build, but there's a good chance that it will. Or that it fails. Hard to say in advance. See, e.g., rust-version-sync@0.8. Greetings, Maxime.
Hello there, On 2022-04-01 08:38, Maxime Devos wrote: >> nom-7.0 is not forward compatible with nom-7.1. I need 7.1 for future >> contributions and this is why I added it into patchset. > > Related: to reduce duplication, you can do Thank you, nice idea. > Also, if semver is used here, nom-7.0 is compatible with nom-7.1 (not > sure about the terminology here -- forwards? backwards?, but updating > it shouldn't cause any problems if semver is followed correctlu). That's right, 7.1 is compatible with 7.0. >> In developer's manifest of nom-7.1 dependency on rust-minimal-lexical >> has been updated from 0.1 to 0.2. For both versions dependencies on >> exact versions on rust-minimal-lexical are pinned by crate developers >> and we cannot change it at will I was wrong, all dependent on nom-7 packages successfully compiled with 7.1. Could you advise on sending a new version of the patches, please? Should I create a completely new patchset with separate patches for each package individually or just one patch with all the fixes is enough now? --
Evgenii Lepikhin schreef op vr 01-04-2022 om 16:51 [+0300]: > Could you advise on sending a new version of the patches, please? > Should I create a completely new patchset with separate patches for > each package individually or just one patch with all the fixes is > enough now? I would send a new patch series with a version number: [PATCH v2 0/7] ..., [PATCH v2 7/7] ..., to the same bug number 54644@debbugs.gnu.org. You can use the -v2 option of "git send-email" for this. If a single ‘fixup patch’ were used, then the commit history would be messier and the reviewer would need to remember which ‘wrong’ things in the earlier patches are fixed in the fixup patch. Greetings, Maxime.
diff --git a/gnu/packages/crates-io.scm b/gnu/packages/crates-io.scm index 3f04192a8a..8cc50adf55 100644 --- a/gnu/packages/crates-io.scm +++ b/gnu/packages/crates-io.scm @@ -34648,6 +34648,23 @@ (define-public rust-miniflux-api-0.3 ;; No copyright headers in the source code. LICENSE indicates gpl3. (license license:gpl3))) +(define-public rust-minimal-lexical-0.2 + (package + (name "rust-minimal-lexical") + (version "0.2.1") + (source + (origin + (method url-fetch) + (uri (crate-uri "minimal-lexical" version)) + (file-name (string-append name "-" version ".tar.gz")) + (sha256 + (base32 "16ppc5g84aijpri4jzv14rvcnslvlpphbszc7zzp6vfkddf4qdb8")))) + (build-system cargo-build-system) + (home-page "https://github.com/Alexhuszagh/minimal-lexical") + (synopsis "Fast float parsing conversion routines") + (description "Fast float parsing conversion routines.") + (license (list license:expat license:asl2.0)))) + (define-public rust-minimal-lexical-0.1 (package (name "rust-minimal-lexical")