mbox series

[bug#38408,0/3] (WIP) Semantic version aware recusive importer for crates

Message ID cover.1574897905.git.mjbecze@riseup.net
Headers show
Series (WIP) Semantic version aware recusive importer for crates | expand

Message

Martin Becze Nov. 28, 2019, 12:13 a.m. UTC
This patch add a new recusive importer (recusive-import-semver) which uses semantic versioning to find the correct version of dependencies. This procedure is then used by the crate importer. Since quite a few langague pms use semantic versioning I hope that other importers can also use it.

This patch has one problem that I'm aware of. recusive-import-semver relies on guile-semver. But how do I make it so that it is installed by default? Or altenativly how can I check a guile module exists or not, so that I can promt the user to install it?

Thanks,
Martin Becze

Martin Becze (3):
  gnu: added new function, find-packages-by-name*/direct
  gnu: added new procedure, recusive-import-semver
  Rewrote some of guix/import/crate.scm to use recursive-import-semver
    and updated script and test.

 gnu/packages.scm              |  41 ++++++++
 guix/import/crate.scm         | 165 +++++++++++++++++--------------
 guix/import/utils.scm         | 181 ++++++++++++++++++++++++++++++++--
 guix/scripts/import/crate.scm |   9 +-
 tests/crate.scm               |   2 +-
 tests/import-utils.scm        | 162 ++++++++++++++++++++++++++++++
 tests/packages.scm            |  13 +++
 7 files changed, 481 insertions(+), 92 deletions(-)

Comments

Martin Becze Dec. 5, 2019, 8:05 p.m. UTC | #1
This version just builds a bit on the prevouse version (https://issues.guix.gnu.org/issue/38408#0) I found while testing  some crates have build-dependencies and nor dependencies that are the same. While in guix build and normal dependiences are treated the same way (ie source only). So the recusive importer was importing the duplicates so here we dedup the deps. Please let me know if there are any problems!

-Martin

Martin Becze (5):
  gnu: added new function, find-packages-by-name*/direct
  gnu: added new procedure, recusive-import-semver
  Rewrote some of guix/import/crate.scm to use recursive-import-semver
    and updated script and test.
  added "#:skip-build? #t" to the output of (make-crate-sexp). Most the
    the packages imported will be libaries and won't need to build. The
    top level package will build them though.
  guix: crate: Depublicated build and normal dependencies

 gnu/packages.scm              |  41 ++++++++
 guix/import/crate.scm         | 188 +++++++++++++++++++---------------
 guix/import/utils.scm         | 181 ++++++++++++++++++++++++++++++--
 guix/scripts/import/crate.scm |   9 +-
 tests/crate.scm               |   5 +-
 tests/import-utils.scm        | 162 +++++++++++++++++++++++++++++
 tests/packages.scm            |  13 +++
 7 files changed, 501 insertions(+), 98 deletions(-)
Martin Becze Dec. 6, 2019, 6:21 p.m. UTC | #2
This version makes one little change from the prevous version (https://issues.guix.gnu.org/issue/38408#13). I found out that crates could have duplicate dependencies in for every possible target in their Cargo.toml. So just depuplicating the build dependencies agains the normal depedencies was not enough. We need to make sure all the dependencies are unqie!

-Martin

Martin Becze (5):
  gnu: added new function, find-packages-by-name*/direct
  gnu: added new procedure, recusive-import-semver
  Rewrote some of guix/import/crate.scm to use recursive-import-semver
    and updated script and test.
  added "#:skip-build? #t" to the output of (make-crate-sexp). Most the
    the packages imported will be libaries and won't need to build. The
    top level package will build them though.
  guix: crate: Depublicated dependencies

 gnu/packages.scm              |  41 ++++++++
 guix/import/crate.scm         | 188 +++++++++++++++++++---------------
 guix/import/utils.scm         | 181 ++++++++++++++++++++++++++++++--
 guix/scripts/import/crate.scm |   9 +-
 tests/crate.scm               |   5 +-
 tests/import-utils.scm        | 162 +++++++++++++++++++++++++++++
 tests/packages.scm            |  13 +++
 7 files changed, 501 insertions(+), 98 deletions(-)
Martin Becze Dec. 10, 2019, 7:23 p.m. UTC | #3
Hi Guix,
This version adds a feature. You can an import a crate using a semantive version range. ie. "guix import crate -r ripgrep@^1". Let me know if you have any suggetions or stuff to fix on this patch! Thanks.

-Martin

Martin Becze (6):
  gnu: added new function, find-packages-by-name*/direct
  gnu: added new procedure, recusive-import-semver
  Rewrote some of guix/import/crate.scm to use recursive-import-semver
    and updated script and test.
  added "#:skip-build? #t" to the output of (make-crate-sexp). Most the
    the packages imported will be libaries and won't need to build. The
    top level package will build them though.
  guix: crate: Depublicated dependencies
  guix: import: recursive-import-semver: allow the range of a package to
    be specified when begining import.

 gnu/packages.scm              |  41 ++++++++
 guix/import/crate.scm         | 186 +++++++++++++++++---------------
 guix/import/utils.scm         | 192 ++++++++++++++++++++++++++++++++--
 guix/scripts/import/crate.scm |   9 +-
 tests/crate.scm               |   5 +-
 tests/import-utils.scm        | 175 +++++++++++++++++++++++++++++++
 tests/packages.scm            |  13 +++
 7 files changed, 522 insertions(+), 99 deletions(-)
Hartmut Goebel Nov. 7, 2020, 10:19 p.m. UTC | #4
Hi Ludo,

this patch is awaiting your review since April. Maybe you could find
some time to finish it, since it would ease importing and updating rust
creates *a lot*. See
https://lists.gnu.org/archive/html/guix-devel/2020-11/msg00168.html

Many thanks.
Marius Bakke Nov. 7, 2020, 10:35 p.m. UTC | #5
Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Hi Ludo,
>
> this patch is awaiting your review since April. Maybe you could find
> some time to finish it, since it would ease importing and updating rust
> creates *a lot*. See
> https://lists.gnu.org/archive/html/guix-devel/2020-11/msg00168.html

I don't think Ludovic has exclusive review rights on this issue.  That
is, anyone is free to review and merge.

If you need this feature, perhaps you could try it and report whether it
works (or not) for you?
Hartmut Goebel Nov. 9, 2020, 5:15 p.m. UTC | #6
Am 07.11.20 um 23:35 schrieb Marius Bakke:
> I don't think Ludovic has exclusive review rights on this issue.  That
> is, anyone is free to review and merge.
>
> If you need this feature, perhaps you could try it and report whether it
> works (or not) for you?

This is a huge pile of comments and versions, patches by others, some 
already applied patches, etc. This is very hard to follow, one has to 
check 20 mails/article whether they contain a patch, same the path 
(giving it a filename), etc. This is why I'm asking those who already 
worked on this to finish the review.

IMHO this shows the limitations of a mail-based patch workflow.

Anyhow, I'll give it a try (steadying my optinion against mail-based 
software development)
Nicolas Goaziou Nov. 9, 2020, 5:27 p.m. UTC | #7
Hello,

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Am 07.11.20 um 23:35 schrieb Marius Bakke:
>> I don't think Ludovic has exclusive review rights on this issue.  That
>> is, anyone is free to review and merge.
>>
>> If you need this feature, perhaps you could try it and report whether it
>> works (or not) for you?
>
> This is a huge pile of comments and versions, patches by others, some
> already applied patches, etc. This is very hard to follow, one has to 
> check 20 mails/article whether they contain a patch, same the path
> (giving it a filename), etc. This is why I'm asking those who already 
> worked on this to finish the review.

IIUC, the base set is there:

 https://lists.gnu.org/archive/html/guix-patches/2020-04/msg01442.html

then, two patches (the first and sixth) need to be replaced with the
following ones:

 https://lists.gnu.org/archive/html/guix-patches/2020-05/msg00408.html

and

 https://lists.gnu.org/archive/html/guix-patches/2020-08/msg00432.html

HTH,
Timothy Sample Nov. 11, 2020, 3:06 p.m. UTC | #8
Hi Hartmut,

Thanks for working on this series!

Hartmut Goebel <h.goebel@crazy-compilers.com> writes:

> Please also make sure, there is a test-case with some 0.x version, since
> crates.io documentations says [1]: "This compatibility convention is
> different from SemVer in the way it treats versions before 1.0.0. …"
>
> [1]
> https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements

As the author of Guile-SemVer, I just wanted to mention that its support
for caret ranges is well-tested and should work well for both NPM and
Cargo.  Caret ranges support the convention that version 0.x.z is
compatible with 0.x.y, where y < z.  The Semantic Versioning document
does not require that compatibility, but I think it’s expected anywhere
Semantic Versioning is used.

For the tests, I suggest having one test that demonstrates that the
importer is relying on Semantic Versioning to resolve packages.  After
that, we can rely on the Guile-SemVer tests to make sure that the
Semantic Versioning resolution will work even for weird edge cases.  To
be clear, I think the all-1.0.0 test is too simplistic.  Maybe the test
should provide packages A@1.2.5 and A@1.3.2, and have the requirement
for package A be “>=1.2.0 <1.3.0”.  It would then make sure that A@1.2.5
is selected.  That way, the test shows that the importer is actually
doing some kind of resolution.  You could use any range syntax, but I
personally find the tildes and carets hard to understand without looking
at the definition.


-- Tim