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.

Sign up for GitHub

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

feat(docs): generate cli docs directly from the struct #12717

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

feat(docs): generate cli docs directly from the struct #12717

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
02f3c9d
refactor the commands to generate the cli docs
akaladarshi Nov 22, 2024
de3e0a8
update the script to generate cli docs directly from command structs
akaladarshi Nov 22, 2024
ed47ad6
add args usage
akaladarshi Nov 25, 2024
e3c452e
refactor: lotus commands
akaladarshi Nov 25, 2024
8233c8e
refactor: lotus-miner commands
akaladarshi Nov 25, 2024
a8b4a75
refactor: cli docs generation
akaladarshi Nov 26, 2024
e542893
update changelog
akaladarshi Nov 26, 2024
7c4702f
address comments
akaladarshi Dec 1, 2024
f616568
refactor the cli doc generation
akaladarshi Dec 1, 2024
bb579bf
refactor the changelog
akaladarshi Dec 1, 2024
e557f34
clean up the doc generation code
akaladarshi Dec 1, 2024
34a9037
load common commands from functions
akaladarshi Dec 3, 2024
4f8d129
Merge branch 'master' into akaladarshi/refactor-docsgen-cli
akaladarshi Dec 3, 2024
f7fe538
use cli package to generate header
akaladarshi Dec 4, 2024
6b8888e
fix remaining diffs between cli docs
akaladarshi Dec 4, 2024
d8f4d19
Merge branch 'master' into akaladarshi/refactor-docsgen-cli
akaladarshi Dec 4, 2024
6a8bef4
fix failing ci
akaladarshi Dec 4, 2024
d7daa99
Merge remote-tracking branch 'origin/master' into akaladarshi/refacto…
akaladarshi Dec 19, 2024
8075d5a
revert command functions to variables
akaladarshi Dec 19, 2024
acf0a33
small fix
akaladarshi Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# UNRELEASED

- Add Market PendingProposals API / CLI. ([filecoin-project/lotus#12724](https://github.com/filecoin-project/lotus/pull/12724))
- Generate the cli docs directly from the code instead compiling and executing binaries' `help` output. ([filecoin-project/lotus#12717](https://github.com/filecoin-project/lotus/pull/12717))

## Improvements

Expand Down
5 changes: 1 addition & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,8 @@ snap: lotus lotus-miner lotus-worker
snapcraft
# snapcraft upload ./lotus_*.snap

# separate from gen because it needs binaries
docsgen-cli: lotus lotus-miner lotus-worker
docsgen-cli:
$(GOCC) run ./scripts/docsgen-cli
./lotus config default > documentation/en/default-lotus-config.toml
./lotus-miner config default > documentation/en/default-lotus-miner-config.toml
Copy link
Member

Choose a reason for hiding this comment

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

@akaladarshi if you want to take a shortcut on this task, we could scope it just to doing what you've done so far - generate the docs from the structs, but leave the existing workflow that requires building the binaries in place, i.e. restore this Makefile target to the way it was. The remaining problem is the need to generate the default config files which currently need the binaries to do it, but we could defer that to a separate PR and get this one landed.

.PHONY: docsgen-cli

print-%:
Expand Down
220 changes: 113 additions & 107 deletions cli/auth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,123 +12,129 @@ import (
"github.com/filecoin-project/lotus/node/repo"
)

var AuthCmd = &cli.Command{
Name: "auth",
Usage: "Manage RPC permissions",
Subcommands: []*cli.Command{
AuthCreateAdminToken,
AuthApiInfoToken,
},
func AuthCmd() *cli.Command {
return &cli.Command{
Name: "auth",
Usage: "Manage RPC permissions",
Subcommands: []*cli.Command{
AuthCreateAdminTokenCmd(),
AuthApiInfoTokenCmd(),
},
}
}

var AuthCreateAdminToken = &cli.Command{
Name: "create-token",
Usage: "Create token",
Flags: []cli.Flag{
&cli.StringFlag{
Name: "perm",
Usage: "permission to assign to the token, one of: read, write, sign, admin",
func AuthCreateAdminTokenCmd() *cli.Command {
return &cli.Command{
Name: "create-token",
Usage: "Create token",
Flags: []cli.Flag{
&cli.StringFlag{
Name: "perm",
Usage: "permission to assign to the token, one of: read, write, sign, admin",
},
},
},

Action: func(cctx *cli.Context) error {
napi, closer, err := GetAPI(cctx)
if err != nil {
return err
}
defer closer()

ctx := ReqContext(cctx)

if !cctx.IsSet("perm") {
return xerrors.New("--perm flag not set")
}

perm := cctx.String("perm")
idx := 0
for i, p := range api.AllPermissions {
if auth.Permission(perm) == p {
idx = i + 1

Action: func(cctx *cli.Context) error {
napi, closer, err := GetAPI(cctx)
if err != nil {
return err
}
defer closer()

ctx := ReqContext(cctx)

if !cctx.IsSet("perm") {
return xerrors.New("--perm flag not set")
}

perm := cctx.String("perm")
idx := 0
for i, p := range api.AllPermissions {
if auth.Permission(perm) == p {
idx = i + 1
}
}
}

if idx == 0 {
return fmt.Errorf("--perm flag has to be one of: %s", api.AllPermissions)
}
if idx == 0 {
return fmt.Errorf("--perm flag has to be one of: %s", api.AllPermissions)
}

// slice on [:idx] so for example: 'sign' gives you [read, write, sign]
token, err := napi.AuthNew(ctx, api.AllPermissions[:idx])
if err != nil {
return err
}
// slice on [:idx] so for example: 'sign' gives you [read, write, sign]
token, err := napi.AuthNew(ctx, api.AllPermissions[:idx])
if err != nil {
return err
}

// TODO: Log in audit log when it is implemented
// TODO: Log in audit log when it is implemented

fmt.Println(string(token))
return nil
},
fmt.Println(string(token))
return nil
},
}
}

var AuthApiInfoToken = &cli.Command{
Name: "api-info",
Usage: "Get token with API info required to connect to this node",
Flags: []cli.Flag{
&cli.StringFlag{
Name: "perm",
Usage: "permission to assign to the token, one of: read, write, sign, admin",
func AuthApiInfoTokenCmd() *cli.Command {
return &cli.Command{
Name: "api-info",
Usage: "Get token with API info required to connect to this node",
Flags: []cli.Flag{
&cli.StringFlag{
Name: "perm",
Usage: "permission to assign to the token, one of: read, write, sign, admin",
},
},
},

Action: func(cctx *cli.Context) error {
napi, closer, err := GetAPI(cctx)
if err != nil {
return err
}
defer closer()

ctx := ReqContext(cctx)

if !cctx.IsSet("perm") {
return xerrors.New("--perm flag not set, use with one of: read, write, sign, admin")
}

perm := cctx.String("perm")
idx := 0
for i, p := range api.AllPermissions {
if auth.Permission(perm) == p {
idx = i + 1

Action: func(cctx *cli.Context) error {
napi, closer, err := GetAPI(cctx)
if err != nil {
return err
}
defer closer()

ctx := ReqContext(cctx)

if !cctx.IsSet("perm") {
return xerrors.New("--perm flag not set, use with one of: read, write, sign, admin")
}
}

if idx == 0 {
return fmt.Errorf("--perm flag has to be one of: %s", api.AllPermissions)
}

// slice on [:idx] so for example: 'sign' gives you [read, write, sign]
token, err := napi.AuthNew(ctx, api.AllPermissions[:idx])
if err != nil {
return err
}

ti, ok := cctx.App.Metadata["repoType"]
if !ok {
log.Errorf("unknown repo type, are you sure you want to use GetCommonAPI?")
ti = repo.FullNode
}
t, ok := ti.(repo.RepoType)
if !ok {
log.Errorf("repoType type does not match the type of repo.RepoType")
}

ainfo, err := GetAPIInfo(cctx, t)
if err != nil {
return xerrors.Errorf("could not get API info for %s: %w", t, err)
}

// TODO: Log in audit log when it is implemented

currentEnv, _, _ := t.APIInfoEnvVars()
fmt.Printf("%s=%s:%s\n", currentEnv, string(token), ainfo.Addr)
return nil
},

perm := cctx.String("perm")
idx := 0
for i, p := range api.AllPermissions {
if auth.Permission(perm) == p {
idx = i + 1
}
}

if idx == 0 {
return fmt.Errorf("--perm flag has to be one of: %s", api.AllPermissions)
}

// slice on [:idx] so for example: 'sign' gives you [read, write, sign]
token, err := napi.AuthNew(ctx, api.AllPermissions[:idx])
if err != nil {
return err
}

ti, ok := cctx.App.Metadata["repoType"]
if !ok {
log.Errorf("unknown repo type, are you sure you want to use GetCommonAPI?")
ti = repo.FullNode
}
t, ok := ti.(repo.RepoType)
if !ok {
log.Errorf("repoType type does not match the type of repo.RepoType")
}

ainfo, err := GetAPIInfo(cctx, t)
if err != nil {
return xerrors.Errorf("could not get API info for %s: %w", t, err)
}

// TODO: Log in audit log when it is implemented

currentEnv, _, _ := t.APIInfoEnvVars()
fmt.Printf("%s=%s:%s\n", currentEnv, string(token), ainfo.Addr)
return nil
},
}
}
12 changes: 6 additions & 6 deletions cli/clicommands/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@ var Commands = []*cli.Command{
lcli.WithCategory("basic", lcli.MultisigCmd),
lcli.WithCategory("basic", lcli.FilplusCmd),
lcli.WithCategory("basic", lcli.PaychCmd),
lcli.WithCategory("developer", lcli.AuthCmd),
lcli.WithCategory("developer", lcli.AuthCmd()),
Copy link
Member

Choose a reason for hiding this comment

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

why do these need to become functions?

Copy link
Contributor Author

@akaladarshi akaladarshi Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Previously, lotus and lotus-miner shared global command variables, causing conflicts when running app.Run("lotus-miner", "-h"). This led to jumbled help output like lotus-miner lotus auth.

Copy link
Member

Choose a reason for hiding this comment

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

See below, I went for a bit of a trek to find out why this is the case and came up with an easier approach than rewriting all of these.

lcli.WithCategory("developer", lcli.MpoolCmd),
lcli.WithCategory("developer", StateCmd),
lcli.WithCategory("developer", lcli.ChainCmd),
lcli.WithCategory("developer", lcli.LogCmd),
lcli.WithCategory("developer", lcli.WaitApiCmd),
lcli.WithCategory("developer", lcli.FetchParamCmd),
lcli.WithCategory("developer", lcli.LogCmd()),
lcli.WithCategory("developer", lcli.WaitApiCmd()),
lcli.WithCategory("developer", lcli.FetchParamCmd()),
lcli.WithCategory("developer", lcli.EvmCmd),
lcli.WithCategory("developer", lcli.IndexCmd),
lcli.WithCategory("network", lcli.NetCmd),
lcli.WithCategory("network", lcli.SyncCmd),
lcli.WithCategory("network", lcli.F3Cmd),
lcli.WithCategory("status", lcli.StatusCmd),
lcli.PprofCmd,
lcli.VersionCmd,
lcli.PprofCmd(),
lcli.VersionCmd(),
}
12 changes: 6 additions & 6 deletions cli/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ var GetStorageMinerAPI = cliutil.GetStorageMinerAPI
var GetWorkerAPI = cliutil.GetWorkerAPI

var CommonCommands = []*cli.Command{
AuthCmd,
LogCmd,
WaitApiCmd,
FetchParamCmd,
PprofCmd,
VersionCmd,
AuthCmd(),
LogCmd(),
WaitApiCmd(),
FetchParamCmd(),
PprofCmd(),
VersionCmd(),
}

func WithCategory(cat string, cmd *cli.Command) *cli.Command {
Expand Down
Loading
Loading