Skip to content

Ls ncdu#4550

Merged
MichaelEischer merged 12 commits intorestic:masterfrom
ndecker:ls-ncdu
Jan 27, 2024
Merged

Ls ncdu#4550
MichaelEischer merged 12 commits intorestic:masterfrom
ndecker:ls-ncdu

Conversation

@ndecker
Copy link
Copy Markdown
Contributor

@ndecker ndecker commented Oct 30, 2023 •
edited by MichaelEischer
Loading

What does this PR change? What problem does it solve?

NCDU (NCurses Disk Usage) is a tool to analyse disk usage of directories.
It has an option to save a directory tree and analyse it later.
This patch adds an output option to the ls command for the NCDU format.

Use restic ls latest --ncdu | ncdu -f -

Was the change previously discussed in an issue or on the forum?

no

Created issue: #4549

Checklist

  • I have read the contribution guidelines.
  • I have enabled maintainer edits.
  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I have run gofmt on the code in all commits.
  • All commit messages are formatted in the same style as the other commits in the repo.
  • I'm done! This pull request is ready for review.

@jvandenbroek
Copy link
Copy Markdown

jvandenbroek commented Nov 27, 2023

I am intrigued by this patch, so had to try it out ;-) But when a snapshot contains multiple paths it doesn't work correctly and specifying a specific path results in the following error:

panic: cannot find suitable parent directory

goroutine 1 [running]:
main.lsNodeNcdu({0x137a120, 0xc00007c030}, 0xc0000f57d0, 0xc0000f5840, {0xc00636cc30, 0xb}, 0xc00636f7c0)
        github.com/restic/restic/cmd/restic/cmd_ls.go:175 +0x505
main.runLs.func6({0xc00636cc30?, 0xb?}, 0xc0005e8720?)
        github.com/restic/restic/cmd/restic/cmd_ls.go:293 +0x45
main.runLs.func9({0x34, 0xc5, 0xfb, 0x92, 0x0, 0x0, 0x15, 0x62, 0x14, 0x5, ...}, ...)
        github.com/restic/restic/cmd/restic/cmd_ls.go:331 +0x90
github.com/restic/restic/internal/walker.walk({0x1387d28, 0xc0000ad5e0}, {0x137a200, 0xc0004b8410}, {0xc00636caa0, 0x4}, {0x34, 0xc5, 0xfb, 0x92, ...}, ...)
        github.com/restic/restic/internal/walker/walker.go:103 +0x2e7
github.com/restic/restic/internal/walker.walk({0x1387d28, 0xc0000ad5e0}, {0x137a200, 0xc0004b8410}, {0x1372ab0, 0x1}, {0x3b, 0xd6, 0x76, 0xc2, ...}, ...)
        github.com/restic/restic/internal/walker/walker.go:122 +0x467
github.com/restic/restic/internal/walker.Walk({0x1387d28, 0xc0000ad5e0}, {0x137a200, 0xc0004b8410}, {0x3b, 0xd6, 0x76, 0xc2, 0x41, 0x28, ...}, ...)
        github.com/restic/restic/internal/walker/walker.go:51 +0x1ef
main.runLs({_, _}, {0x0, {{}, {0x0, 0x0, 0x0}, {0x0, 0x0, 0x0}, ...}, ...}, ...)
        github.com/restic/restic/cmd/restic/cmd_ls.go:321 +0x78d
main.glob..func15(0xc00019b500?, {0xc0003e9340?, 0x4?, 0x11afcb5?})
        github.com/restic/restic/cmd/restic/cmd_ls.go:47 +0x87
github.com/spf13/cobra.(*Command).execute(0x1b49f00, {0xc0003e92d0, 0x7, 0x7})
        github.com/spf13/cobra@v1.7.0/command.go:940 +0x87c
github.com/spf13/cobra.(*Command).ExecuteC(0x1b4e6e0)
        github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3a5
github.com/spf13/cobra.(*Command).Execute(...)
        github.com/spf13/cobra@v1.7.0/command.go:992
github.com/spf13/cobra.(*Command).ExecuteContext(...)
        github.com/spf13/cobra@v1.7.0/command.go:985
main.main()
        github.com/restic/restic/cmd/restic/main.go:109 +0x1a5
Error importing file on line 2:2: expected ','.

@MichaelEischer MichaelEischer mentioned this pull request Jan 6, 2024
Merged
7 tasks
@MichaelEischer
Copy link
Copy Markdown
Member

MichaelEischer commented Jan 21, 2024 •
edited
Loading

I've extended the walker to also inform about leaving a directory. This allows significantly simplifying the JSON printing code. The different output formats are now also abstracted using a common interface such that no more workarounds are necessary to print the NCDU json.

@jvandenbroek The code should now also work when specifying a path.

remaining TODOs:

  • add tests for walker.LeaveDir
  • add documentation

ndecker and others added 12 commits January 27, 2024 13:06
@ndecker @MichaelEischer
command ls: add option for ncdu output
a931094 
NCDU (NCurses Disk Usage) is a tool to analyse disk usage of directories.
It has an option to save a directory tree and analyse it later.
This patch adds an output option to the ls command.

A snapshot can be seen with

`restic ls latest --ncdu | ncdu -f -`

- https://dev.yorhel.nl/ncdu
@ndecker @MichaelEischer
add changelog for ls --ncdu
b2703a4
@MichaelEischer
walker: add callback to inform about leaving a directory
9ecbda0
@MichaelEischer
ls: rework ncdu output to use walker.LeaveDir
1b008c9
@MichaelEischer
ls: unify printer implementations
a2fe337
@MichaelEischer
ls: correctly handle setuid/setgit/sticky bit in ncdu output
509b339
@MichaelEischer
ls: test ncdu output format
4bae54d
@MichaelEischer
fix linter warning
a2f2f8f
@MichaelEischer
ls: only allow either --json or --ncdu
261737a
@MichaelEischer
walker: fix missing leaveDir if directory is partially skipped
2c80cfa
@MichaelEischer
walker: add tests for leaveDir
d4ed7c8
@MichaelEischer
describe ls command in docs
10e71af
MichaelEischer
MichaelEischer approved these changes Jan 27, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM.

@MichaelEischer MichaelEischer added this pull request to the merge queue Jan 27, 2024
Merged via the queue into restic:master with commit e44e4b0 Jan 27, 2024
@MichaelEischer MichaelEischer mentioned this pull request Feb 8, 2024
Closed
@skywinder
Copy link
Copy Markdown

skywinder commented Feb 27, 2024 •
edited
Loading

So cool feature, thanks! Will it be released in the next version?

@MichaelEischer
Copy link
Copy Markdown
Member

MichaelEischer commented Feb 27, 2024

Yes it will be part of restic 0.17, but that will still take a few months until it will be released. Until then you can use a beta release from https://beta.restic.net/?sort=time&order=desc .

@akrabu
Copy link
Copy Markdown

akrabu commented Jul 7, 2024 •
edited
Loading

@MichaelEischer When testing this with snapshots of multiple paths, it seems to only pass the first folder to NCDU - unless I'm doing something wrong? I see @jvandenbroek mentioned this already. Is the answer that only the first path is supported by default, and if you want another path you must specify it - or will it be possible to have it consider all paths by default eventually?

@MichaelEischer MichaelEischer mentioned this pull request Jul 7, 2024
Closed
@MichaelEischer
Copy link
Copy Markdown
Member

MichaelEischer commented Jul 7, 2024

@akrabu In theory multiple paths should work just fine. I've fixed the crash and had hoped that multiple paths work. But apparently there's something broken, see #4910.

@BrewTestBot BrewTestBot mentioned this pull request Jul 26, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaelEischer MichaelEischer MichaelEischer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ndecker @jvandenbroek @MichaelEischer @skywinder @akrabu