implement cache based listing #29

amtoine · 2023-10-20T13:36:52Z

description

#26 had three major flaws, list-repos-in-store

did depend on GNU find on Linux and MacOS
did use glob on Windows, which is slow
did not catch any bare repo and did list nested directories, such as Git modules

this PR tries to solve these at once

will use glob only for all the OSes and removed the dependency on find
will use a cache to store the state of the repos currently managed by gm
the globbing pattern and post-treatment will both catch the bare repos and remove nested duplicates

new features

gm cache is a new command that gives the cache currently used
- the cache will follow the XDG Base Directory specification and will default to ~/.local/share/nu-git-manager/cache.nuon
- it has a --update switch to force a full update of the cache: this might take some time
gm list will simply open the cache and dump it's content as a list: this will be super fast
gm clone and gm remove will update the cache in a super fast way, without list-repos-in-store
- gm clone will just append the new path to the cache
- gm remove will just remove the removed path from the cache
- otherwise, if the user changes the store manually, they will have to run gm cache --update manually to fix gm list's output
gm list will be used in other commands instead of list-repos-in-store to use the cache

Note
paths will be more sanitized by removing the drive, e.g. D:, from the beginning of Windows paths

Note
list-repos-in-store will throw a DEBUG line when the store does not exist.
the CI will set NU_LOG_LEVEL to DEBUG to see that in the CI.

Note
this PR makes sure all paths are sanitized and removes mention to path_sep from the gm module

details on the changes

with this PR, list-repos-in-store will

use glob
search for any occurence of a HEAD file
remove invalid matches, e.g. multiple HEADs that are stored in multiple places in .git/ directories
remove nested duplicates from the list, e.g. Git submodules

tests

this PR adds two tests

get-repo-cache to make sure the cache is properly setup
list-all-repos-in-store to make sure list-repos-in-store can list all repos, bare ones included, without nested duplicates, in a fake store

a real-life example would be the `nushell` and `nushell.github.io` repos: we want them both.

a real-life example would be the `nushell` and `nushell.github.io` repos: before this change, `nushell.github.io` would be discarded by `list-repos-in-store` because `nushell.github.io` starts with `nushell`. keeping the trailing `/` in all paths fixes the error because `nushell.github.io/` does not start with `nushell/` anymore 👌

because the paths are sanitized, they will all use `/`

amtoine · 2023-10-20T14:44:56Z

Note
might need to add back some more --not to the glob call

related to - amtoine/nu-git-manager#29 will require to install `nu-git-manager` with Nupm from amtoine/nu-git-manager#29.

all paths should be sanitized now

amtoine · 2023-10-20T15:36:02Z

i've added NOTEs in the PR description about fixing this PR's CI for Windows, hope that's clear enough 👀 🤞

nu-git-manager/fs/store.nu

melMass · 2023-10-21T00:59:43Z

Nice!!

gm cache is a new command that gives the cache currently used

I don't see this exposed?

edit: the whole module silently failed to load actually... I see it in code but it wasn't working in the cli

gm clone and gm remove will run gm list --update to update the cache

Wouldn't that be super slow? (I still have to test it out)

Not sure what it is, maybe the imports, I'll check that

amtoine · 2023-10-21T11:28:40Z

I don't see this exposed?

edit: the whole module silently failed to load actually... I see it in code but it wasn't working in the cli

use ./nu-git-manager/ "gm cache"

works on my side 👍

Wouldn't that be super slow? (I still have to test it out)

it will very likely be much slower than just updating the cloned / removed project 🤔
i'll work on that 👍

Not sure what it is, maybe the imports, I'll check that

if gm cache hasn't been brought into scope that's expected 😉

i'll work on a little toolkit to simplify that 😌

when cloning or removing a project, there is no need to update the whole cache, that takes unnecessary time... this commit only appends the new project to the cache or removes it when using `gm clone` or `gm remove`. because it started to be used in multiple places, i've refactored the check of the cache existence in `check-cache-file` in the `fs/store/` module.

amtoine · 2023-10-21T11:47:32Z

Wouldn't that be super slow? (I still have to test it out)

that should be muuuuuch faster anyways thanks to fa3f5ea 🤞

melMass · 2023-10-21T11:57:42Z

works on my side 👍

Yep this was before I understand that all commands are imported individually, I added it to the readme here b12bd94 I wasn't sure because it was only used to get the cache path but with your new commit it make sense imo to mention it in the import examples

that should be muuuuuch faster anyways thanks to fa3f5ea 🤞

Awesome!

amtoine · 2023-10-21T12:03:18Z

Yep this was before I understand that all commands are imported individually, I added it to the readme here b12bd94 I wasn't sure because it was only used to get the cache path but with your new commit it make sense imo to mention it in the import examples

oooh ok ok 👍
you can also run

use nu-git-manager *

😋

Awesome!

tell me when the changes are ok, or feel free to land if there's nothing more to add right now 😌

melMass · 2023-10-24T12:19:08Z

Small detail (as it works fine) but there is still a backslash there:

melMass · 2023-10-24T12:35:18Z

There is no much point in my addition to the readme, I reverted it

melMass

🚀

amtoine · 2023-10-24T16:07:30Z

thanks for landing this 🙏

this has been introduced in #29, likely through an annoying and automatic formatting of the Markdown document 😮

amtoine added 11 commits October 20, 2023 15:07

add a test to make sure all repos are listed

e107264

add gm list --update to update the cache of repos

09883a6

use gm list and update cache on clone and remove

f6992ec

add gm cache

0436fad

add a test for getting the cache path

18d0013

remove nested repositories

fc52fa5

remove find from the dependencies

fc30873

add a test to make sure foo.bar and foo are both listed

c3213ca

a real-life example would be the `nushell` and `nushell.github.io` repos: we want them both.

be more explicit in the list-all-repos-in-store test

2badc7e

use / in the tests directly

c11f4b7

because the paths are sanitized, they will all use `/`

amtoine requested a review from melMass October 20, 2023 13:37

amtoine added 3 commits October 20, 2023 16:20

sanitize the path to the cache

396b0cf

fix the get-repo-cache test by sanitizing paths

25d02ee

remove drive from Windows paths when sanitizing

dd31234

amtoine added 3 commits October 20, 2023 16:51

cd into the store before globbing

1ef49ec

do not use hardcoded $env.GIT_REPOS_STORE

6ce80ee

make the FIXME about *globbing* more clear

8130316

amtoine added a commit to amtoine/dotfiles that referenced this pull request Oct 20, 2023

bring gm cache to scope from nu-git-manager

f575349

related to - amtoine/nu-git-manager#29 will require to install `nu-git-manager` with Nupm from amtoine/nu-git-manager#29.

amtoine added 7 commits October 20, 2023 17:09

add a debug statement when the repo store does not exist

9dff625

set the log level of Nushell to "DEBUG" in the CI

a1fab43

sanitize the $BASE of the fake store in tests

0c76ca1

add a note about $BASE not being a const

616f964

sanitize the paths out of list-repos-in-store

3af06bc

remove last mentions to path_sep

87b5970

all paths should be sanitized now

sanitize the listed paths before remove the HEAD

39af729

melMass reviewed Oct 21, 2023

View reviewed changes

nu-git-manager/fs/store.nu Show resolved Hide resolved

docs: add cache to the readme import instructions

b12bd94

amtoine added 2 commits October 21, 2023 13:33

move the cache update to gm cache

f6e1d6b

minor formatting

bc9b8f9

chore: revert b12bd94

c8af081

melMass approved these changes Oct 24, 2023

View reviewed changes

melMass merged commit f454d80 into main Oct 24, 2023
3 checks passed

melMass deleted the cache-based-listing branch October 24, 2023 12:37

amtoine mentioned this pull request Oct 28, 2023

test coverage #24

Closed

4 tasks

amtoine mentioned this pull request Jan 22, 2024

fix the table of content #161

Merged

amtoine added a commit that referenced this pull request Jan 22, 2024

fix the table of content (#161)

5044035

this has been introduced in #29, likely through an annoying and automatic formatting of the Markdown document 😮

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement cache based listing #29

implement cache based listing #29

amtoine commented Oct 20, 2023 •

edited

Loading

amtoine commented Oct 20, 2023

amtoine commented Oct 20, 2023 •

edited

Loading

melMass commented Oct 21, 2023 •

edited

Loading

amtoine commented Oct 21, 2023

amtoine commented Oct 21, 2023

melMass commented Oct 21, 2023

amtoine commented Oct 21, 2023

melMass commented Oct 24, 2023

melMass commented Oct 24, 2023

melMass left a comment

amtoine commented Oct 24, 2023

implement cache based listing #29

implement cache based listing #29

Conversation

amtoine commented Oct 20, 2023 • edited Loading

description

new features

details on the changes

tests

amtoine commented Oct 20, 2023

amtoine commented Oct 20, 2023 • edited Loading

melMass commented Oct 21, 2023 • edited Loading

amtoine commented Oct 21, 2023

amtoine commented Oct 21, 2023

melMass commented Oct 21, 2023

amtoine commented Oct 21, 2023

melMass commented Oct 24, 2023

melMass commented Oct 24, 2023

melMass left a comment

Choose a reason for hiding this comment

amtoine commented Oct 24, 2023

amtoine commented Oct 20, 2023 •

edited

Loading

amtoine commented Oct 20, 2023 •

edited

Loading

melMass commented Oct 21, 2023 •

edited

Loading