coffee-tools / coffee

Reference implementation for a flexible core lightning plugin manager
https://docs.page/coffee-tools/coffee
9 stars 13 forks source link

feat(core): make a repository dir for each network #239

Open vincenzopalazzo opened 7 months ago

vincenzopalazzo commented 7 months ago

This is the more sane thing to do because each network can have a different repository version.

Fixes: https://github.com/coffee-tools/coffee/issues/234 Fixes https://github.com/coffee-tools/coffee/issues/233

netlify[bot] commented 7 months ago

Deploy Preview for coffee-docs canceled.

Name Link
Latest commit 8615159f5cc5edfd74044354dd99e7675835a00a
Latest deploy log https://app.netlify.com/sites/coffee-docs/deploys/66447145c3ccc500087ded41
vincenzopalazzo commented 7 months ago

ok I pushed the review you suggested @tareknaser tomorrow I will take a look if this PR is completed, but I guess yes?

I m not sure just when run the nurse command

tareknaser commented 7 months ago

just a reminder that https://github.com/coffee-tools/coffee/pull/239#pullrequestreview-1873635869 is still not resolved

vincenzopalazzo commented 5 months ago

Ok I think we must complete this, I ran into some issues that are really embracing

So, if we run coffee nurse and then see the coffee remote list for a network. is the output sane?

Yes, why they should not be sane @tareknaser? I am missing something around your point?

vincenzopalazzo commented 5 months ago

ok fixed a couple of bugs found with the nurse command.

Now the nurse command will migrate the directory, and then restore the repositories to have a fresh copy of them 😄

Review are welcome

vincenzopalazzo commented 5 months ago

With the current code I had to run two times the nurse command to have all fixed

➜  ~ coffee nurse
╭────────────────────────────────────────────────────────────────╮
│ ●   Actions Taken                        Affected repositories │
├────────────────────────────────────────────────────────────────┤
│ ●   Moving Global repository directory   testnet               │
│ ●   Moving Global repository directory   bitcoin               │
╰────────────────────────────────────────────────────────────────╯
➜  ~ coffee remote list
Error: CoffeeError { code: 1, msg: "Coffee found some defects in the configuration. Please run `coffee nurse` to fix them.\n                    If you are want to skip the verification, please add the `--skip-verify ` flag to the command." }
➜  ~ coffee nurse
╭─────────────────────────────────────────────────────╮
│ ●   Actions Taken        Affected repositories      │
├─────────────────────────────────────────────────────┤
│ ●   Restored using Git   summars, teos, folgore-git │
╰─────────────────────────────────────────────────────╯
➜  ~ coffee remote list
● List of repositories
╭───────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ ●   Repository Alias   URL                                            N. Plugins   Git HEAD   Last Update │
├───────────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ●   folgore-git        https://github.com/coffee-tools/folgore        1            aedd703    12/03/2024  │
│ ●   summars            https://github.com/daywalker90/summars         1            c73633a    18/03/2024  │
│ ●   teos               https://github.com/vincenzopalazzo/rust-teos   1            507b8e2    09/04/2024  │
╰───────────────────────────────────────────────────────────────────────────────────────────────────────────╯

Maybe this is a way to do stuff but not sure if it is so good from implementation view point

vincenzopalazzo commented 5 months ago

It breaks the upgrade command

➜  ~ coffee upgrade folgore-git -v
HEAD is now at 09189fb deps: changing the git url for esplora
cp: cannot stat '/home/vincent/.coffee/repositories/folgore-git': No such file or directory
✗ Error: code: 2, msg:
tareknaser commented 5 months ago

Currently, remote repositories are still global in the coffee configuration, not specific to individual networks. To reproduce:

coffee remote add lightningd https://github.com/lightningd/plugins.git
coffee -n testnet remote add folgore https://github.com/coffee-tools/folgore.git 
coffee remote list 
coffee -n testnet remote list
coffee install folgore # Shouldn't work
coffee remote rm folgore
coffee -n testnet remote list

Expected

Output for coffee remote list: lightningd Output for coffee -n testnet remote list: folgore

Output

tareknasser@Tareks-MacBook ~ % coffee remote add lightningd https://github.com/lightningd/plugins.git
✓ Remote added!
tareknasser@Tareks-MacBook ~ % coffee -n testnet remote add folgore https://github.com/coffee-tools/folgore.git 
✓ Remote added!
tareknasser@Tareks-MacBook ~ % coffee remote list
● List of repositories
╭──────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ ●   Repository Alias   URL                                       N. Plugins   Git HEAD   Last Update │
├──────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ●   folgore            https://github.com/coffee-tools/folgore   1            09189fb    20/03/2024  │
│ ●   lightningd         https://github.com/lightningd/plugins     19           fef7ded    12/04/2024  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────╯
tareknasser@Tareks-MacBook ~ % coffee -n testnet remote list
● List of repositories
╭──────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ ●   Repository Alias   URL                                       N. Plugins   Git HEAD   Last Update │
├──────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ●   lightningd         https://github.com/lightningd/plugins     19           fef7ded    12/04/2024  │
│ ●   folgore            https://github.com/coffee-tools/folgore   1            09189fb    20/03/2024  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────╯
tareknasser@Tareks-MacBook ~ % coffee install folgore                 
✓ Compiling and installing
✓ Plugin folgore Compiled and Installed
tareknasser@Tareks-MacBook ~ % coffee remote rm folgore
✓ Remote removed!
tareknasser@Tareks-MacBook ~ % coffee -n testnet remote list
● List of repositories
╭────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ ●   Repository Alias   URL                                     N. Plugins   Git HEAD   Last Update │
├────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ●   lightningd         https://github.com/lightningd/plugins   19           fef7ded    12/04/2024  │
╰────────────────────────────────────────────────────────────────────────────────────────────────────╯
vincenzopalazzo commented 5 months ago

I think your case can be written in some integration testing that we have, so looking in doing it

vincenzopalazzo commented 5 months ago

Thanks for this amazing catch @tareknaser

I decided to fix our tests and write your example case in our test suite, to run it, run just make integration ARGS="-- --test-threads=4 test_migrated_repository_to_local_one"

P.S: pushed the test in a single single commit, and I find also a bug inside the nurse strategy that was not checking if there was the root repositories dir.

So now we should be ok

vincenzopalazzo commented 5 months ago

Find a bug when I try to add two times the same repo but in different network

➜  ~ coffee remote list
● List of repositories
╭──────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ ●   Repository Alias   URL                                       N. Plugins   Git HEAD   Last Update │
├──────────────────────────────────────────────────────────────────────────────────────────────────────┤
│ ●   folgore-git        https://github.com/coffee-tools/folgore   1            09189fb    20/03/2024  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────╯
➜  ~ coffee remote add folgore-git https://github.com/coffee-tools/folgore.git
✗ Fetch remote from https://github.com/coffee-tools/folgore.git error: Error while add remote: code: 1, msg: repository with name: folgore-git already exists
✗ Error: code: 1, msg: repository with name: folgore-git already exists
tareknaser commented 5 months ago

Find a bug when I try to add two times the same repo but in different network

Good point. We need to modify coffee.add_remote() so it checks against remotes specific to the network instead of global repositories

vincenzopalazzo commented 5 months ago

Good point. We need to modify coffee.add_remote() so it checks against remotes specific to the network instead of global repositories

Thanks for suggesting the fix, I will try to look at this tomorrow :)

vincenzopalazzo commented 4 months ago

MH we should push this forward!