Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow online installation of packages #56

Merged
merged 26 commits into from
Feb 18, 2024
Merged

Conversation

kubouch
Copy link
Contributor

@kubouch kubouch commented Dec 28, 2023

Registries

See the newly added registry.nuon file for more specific info about the registry format.

This PR adds a new concept: registry. Registries are .nuon files containing a list of packages, their versions, and how they should be installed. Currently, registry can contain two kinds of packages:

  • local -- installed from local file system
  • git -- installed from remote Git repository

The list of recognized registries is stored in $env.NUPM_REGISTRIES as a record of name: path pairs. That way, users can maintain their collection of sources from where they want to install packages. Not sure if this will be the final way to pass available registries to nupm, but should be good enough for early testing.

Online Installation

If a package is to be installed from a Git repository, the repository is cached in $env.NUPM_CACHE to avoid re-cloning the same repository all the time. Once cloned, the local path of the cloned repository is passed to the installer, as if you called nupm install --path=....

One important missing feature is some kind of file integrity check. One step at a time...

Versions

If a registry contains multiple versions (common scenario), by default, the newest one is installed. "Newest" currently means sorting the versions alphabetically and taking the last one. If --pkg-version is specified, nupm tries to install the exact version passed to this option. This needs better version matching (e.g., version 0.2 matching 0.2.1 etc.), but again, one step at a time...

New command

nupm search searches for packages in registries without installing them. I thought this would be useful to have early on.

Try it out

From the repository root:

overlay use nupm/
nupm install nu-git-manager
# use nu-git-manager *
# ...

TODO

  • Remove debug stuff
  • Nicer / more informative prints
  • (can be done over time) Rewrite https://github.com/nushell/nupm/blob/main/index.nuon to fit the registry format, move it to its own repo
    • It would be a single-file repository
    • Package authors would put PRs there whenever they update their package
  • Tests

After

  • better version matching => 0.2 shouldn't match 0.2.1
  • file integrity checks

@kubouch kubouch requested a review from amtoine December 28, 2023 14:57
@AucaCoyan
Copy link

Poggers! This is really cool! Good job kobouch! 🎉

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without looking at the changes, i'm not able to make the commands in Try it out work 🤔

Note
i've added a use ./nupm/ to load Nupm before running nupm install

  • installing from the kubouch registry
nupm install spam_script --registry kubouch
https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon
Error:   × No package matching version ``
  • installing from the local registry
nupm install spam_script --registry local_test --force
/home/amtoine/.local/share/nupm/modules/tests/packages/registry.nuon
Error: nu::shell::eval_block_with_input

  × Eval block failed with pipeline input
   ╭─[entry #2:1:1]
 1 │ ╭─▶ $env.NUPM_REGISTRIES = {
 2 │ │       kubouch: 'https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon'
 3 │ │       local_test: ($env.FILE_PWD | path join tests packages registry.nuon)
 4 │ ├─▶ }
   · ╰──── source value
   ╰────

Error:   × Cannot open '/home/amtoine/.local/share/nupm/modules/tests/packages/registry.nuon' as a file or URL.
  • giving a package version
nupm install spam_script --registry local_test --pkg-version
Error: nu::parser::missing_flag_param

  × Missing flag argument.
   ╭─[entry #6:1:1]
 1 │ nupm install spam_script --registry local_test --pkg-version
   ·                                                ──────┬──────
   ·                                                      ╰── flag missing string argument
   ╰────

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

i could run the first step with

nupm install spam_script --registry kubouch --pkg-version '0.1.0'

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

Ah, you managed to find a Nushell type checking bug, I fixed the missing --pkg-version error :-).

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

Seems like you're trying to run it from the installed directory, the test commands in the PR description are meant to be run from the nupm repo root . The tests/ directory shouldn't be installed, so it makes sense it doesn't work. I'll update the description.

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

Ah, you managed to find a Nushell type checking bug, I fixed the missing --pkg-version error :-).

ehe 😏

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

nice the first one is fixed 👌

but the two other ones are not 🤔

Seems like you're trying to run it from the installed directory, the test commands in the PR description are meant to be run from the nupm repo root . The tests/ directory shouldn't be installed, so it makes sense it doesn't work. I'll update the description.

> pwd
/home/amtoine/documents/repos/github.com/amtoine/nushell-nupm

and

nupm install spam_script --registry local_test --force

and

nupm install spam_script --registry local_test --pkg-version 0.1.0 --force

both give me

Error: nu::shell::eval_block_with_input

  × Eval block failed with pipeline input
    ╭─[/home/amtoine/documents/repos/github.com/amtoine/nushell-nupm/nupm/mod.nu:25:1]
 25 │         #       file (requires nuon formatting).
 26 │ ╭─▶     $env.NUPM_REGISTRIES = {
 27 │ │           # nupm: ($env.NUPM_HOME | path join registry.nuon)
 28 │ │           kubouch:'https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon'
 29 │ │           local_test: ($env.FILE_PWD | path join tests packages registry.nuon)
 30 │ │           # remote_test: 'https://raw.githubusercontent.com/nushell/nupm/main/tests/packages/registry.nuon'
 31 │ ├─▶     }
    · ╰──── source value
 32 │     }
    ╰────

Error:   × Cannot open '/home/amtoine/.local/share/nupm/modules/tests/packages/registry.nuon' as a file or URL.

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

I also reordered and fixed the steps, check it out. Save the whole snippet to a file and run nu file.nu from the repo root.

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

I also reordered and fixed the steps, check it out. Save the whole snippet to a file and run nu file.nu from the repo root.

ooh interesting

  • when i run from inside a script, i get no errors, but the middle print is Hello world!, not Hello world v0.2.0!
  • when i run the same steps from the REPL, i get the error above with the two last nupm install commands

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

The first bullet should work now.

The second bullet is because you have misconfigured the registry path (note, I changed FILE_PWD to PWD). It's just for the sake of the example, you can define your own paths.

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

yup, the first bullet is fixed 👌

as for the second one, i'm not sure what i should run in the REPL to make it work...
i have this script for now

$env.NUPM_HOME = /tmp/nupm

$env.NUPM_REGISTRIES = {
    kubouch: 'https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon'
    local_test: ($env.PWD | path join tests packages registry.nuon)
}

# Make installed scripts executable
$env.PATH ++= [ ($env.NUPM_HOME | path join scripts) ]

use nupm

nupm install spam_script --registry kubouch --force
spam_script.nu # prints "Hello world!"

nupm install spam_script --registry local_test --force
spam_script.nu # prints "Hello world v0.2.0!"

nupm install spam_script --registry local_test --pkg-version 0.1.0 --force
spam_script.nu # prints "Hello world!"

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

You can run each line/statement as its own REPL iteration. Note that you've set NUPM_HOME to a file, not a directory.

You might have some old stuff lingering around from the previous REPL attempts. Try running a fresh nu -n.

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

You can run each line/statement as its own REPL iteration. Note that you've set NUPM_HOME to a file, not a directory.

yeah my bad that was a typo...

You might have some old stuff lingering around from the previous REPL attempts. Try running a fresh nu -n.

yes i did run everything above (with the typo fixed) from nu -n and still get the same error

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

but there is something fishy going on

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

before running the first nupm install

> $env.NUPM_REGISTRIES
╭───────────────┬──────────────────────────────────────────────────────────────────────────────────────────────╮
│ kubouch       │ https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon                                    │
│ local_test    │ /home/amtoine/documents/repos/github.com/amtoine/nushell-nupm/tests/packages/registry.nuon   │
╰───────────────┴──────────────────────────────────────────────────────────────────────────────────────────────╯

and after

╭────────────┬───────────────────────────────────────────────────────────╮
│ kubouch    │ https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon │
│ local_test │ /home/amtoine/.config/bspwm/tests/packages/registry.nuon  │
╰────────────┴───────────────────────────────────────────────────────────╯

i run BSPWM with Nushell, no idea what it has to do with Nupm here 👀

@kubouch
Copy link
Contributor Author

kubouch commented Dec 29, 2023

That's because NUPM_REGISTRIES are currently set to some debugging values, do not run it with nupm install, just set the values manually and run it locally. I just wanted to get feedback on the core functionality before moving on.

Not sure what's going on with bspwm, but there are still some debug hardcoded values like that that will be removed.

@amtoine
Copy link
Member

amtoine commented Dec 29, 2023

aah yeah, sorry again for the back and forth @kubouch, it works fine both in a script and in the REPL after i apply

diff --git a/nupm/mod.nu b/nupm/mod.nu
index 3b7b0de..53152bd 100644
--- a/nupm/mod.nu
+++ b/nupm/mod.nu
@@ -26,7 +26,7 @@ export-env {
     $env.NUPM_REGISTRIES = {
         # nupm: ($env.NUPM_HOME | path join registry.nuon)
         kubouch:'https://git.sr.ht/~kubouch/nupkgs/blob/main/registry.nuon'
-        local_test: ($env.FILE_PWD | path join tests packages registry.nuon)
+        local_test: ("/home/amtoine/documents/repos/github.com/amtoine/nushell-nupm" | path join tests packages registry.nuon)
         # remote_test: 'https://raw.githubusercontent.com/nushell/nupm/main/tests/packages/registry.nuon'
     }
 }

@kubouch kubouch force-pushed the nupm-registry2 branch 3 times, most recently from 3e8ff6b to 6f4a43a Compare January 28, 2024 12:41
@kubouch
Copy link
Contributor Author

kubouch commented Jan 28, 2024

OK, I think it's ready for review. Could be polished and tested more thoroughly, but then it would never be ready :-D

@kubouch kubouch marked this pull request as ready for review January 28, 2024 16:43
@kubouch kubouch requested a review from amtoine January 28, 2024 16:43
Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okey, i've had a look at the tests and they look great.

before moving on to a quick look at the code, i can report you @kubouch that running

use ./nupm/

and then

nupm search nu-git-manager

gives

Error: nu::shell::eval_block_with_input

  × Eval block failed with pipeline input
    ╭─[/home/amtoine/documents/repos/github.com/amtoine/nushell-nupm/nupm/utils/registry.nu:28:1]
 28 │     # Collect all registries matching the package and all matching packages
 29 │     let regs = $registries
    ·                ─────┬─────
    ·                     ╰── source value
 30 │         | items {|name, path|
    ╰────

Error:   × Cannot open '/home/amtoine/.local/share/nupm/modules/registry.nuon' as a file or URL.

and

nupm install --no-confirm nu-git-manager

gives

Error: nu::shell::eval_block_with_input

  × Eval block failed with pipeline input
    ╭─[/home/amtoine/documents/repos/github.com/amtoine/nushell-nupm/nupm/utils/registry.nu:28:1]
 28 │     # Collect all registries matching the package and all matching packages
 29 │     let regs = $registries
    ·                ─────┬─────
    ·                     ╰── source value
 30 │         | items {|name, path|
    ╰────

Error:   × Cannot open '/home/amtoine/.local/share/nupm/modules/registry.nuon' as a file or URL.

@kubouch
Copy link
Contributor Author

kubouch commented Jan 29, 2024

That's because the registry is pointing at the local registry.nuon which doesn't get installed by nupm. This will be pointing at the online file, but because it's not merged yet, I put the local file there for now. Maybe I should commit the registry separetely?

Try running it from the repo root without installing nupm. It's a chicken and egg problem...

@amtoine
Copy link
Member

amtoine commented Jan 31, 2024

@kubouch
i don't know why it keeps not working...

what i'm seeing is that the $path that http get tries to fetch is a local path on my filesystem and thus it fails and gives the error above 🤔

@amtoine
Copy link
Member

amtoine commented Jan 31, 2024

i'm really just running the two commands i mention above 😕

@kubouch
Copy link
Contributor Author

kubouch commented Jan 31, 2024

nupm tries http get $path if $path is not a valid file. Which means that the nupm_test registry pointed at invalid file. This might be because you installed nupm with nupm which doesn't install the registry by default and because the registry was derived from $env.FILE_PWD.

Now, I changed the default registry to point at nupm repository so it should work regardless of how you installed nupm, one #68 is merged.

kubouch added a commit that referenced this pull request Jan 31, 2024
<!-- related issues, e.g. "will close #123" -->

## Description
<!-- describe in a few words the changes -->

This adds a registry for nupm packages following a format proposed in
#56.

I'm making a separate PR to allow testing
#56 before merging it.
@amtoine
Copy link
Member

amtoine commented Feb 1, 2024

aah yeah, let me try again 👍

@amtoine
Copy link
Member

amtoine commented Feb 1, 2024

okey, the example appears to work now 🥳

i won't have time to finish this right now, i'll come back to it soon 🙏

Copy link
Member

@amtoine amtoine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okey, the code looks mostly fine to me, especially as the commands appear to be working fine!

i just have a minor question, but feel free to land this regardless 😉
thanks @kubouch for working on this 🙏

nupm/utils/registry.nu Show resolved Hide resolved
@amtoine
Copy link
Member

amtoine commented Feb 18, 2024

let's try it out and see how it goes 😋

@amtoine amtoine merged commit 29916fc into nushell:main Feb 18, 2024
3 checks passed
@kubouch kubouch mentioned this pull request Feb 18, 2024
4 tasks
kubouch pushed a commit that referenced this pull request Feb 18, 2024
followup to #56 

## Description
this should update the `registry.nuon` file thanks to the content of
previous `index.nuon` (see cbca144 for the pipeline used to update the
registry).

this PR uses the latest version of each package.

## try it out
```nushell
nupm search --registry ./registry.nuon explore
    | into record
    | update pkgs { into record }
    | table --expand
```
```
────┬──────────────────────────────────────────────────────
name│registry
path│./registry.nuon
    │────────┬─────────────────────────────────────────────
pkgs│name    │nu_plugin_explore
    │version │0.1.2
    │url     │https://github.com/:amtoine/nu_plugin_explore
    │revision│0.1.2
    │path    │.
    │type    │git
    │────────┴─────────────────────────────────────────────
────┴──────────────────────────────────────────────────────
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants