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

feat(version): Add extraInfo to version cmd #18063

Merged
merged 15 commits into from
Oct 30, 2023
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (rpc) [#17470](https://github.com/cosmos/cosmos-sdk/pull/17470) Avoid open 0.0.0.0 to public by default and add `listen-ip-address` argument for `testnet init-files` cmd.
* (types) [#17670](https://github.com/cosmos/cosmos-sdk/pull/17670) Use `ctx.CometInfo` in place of `ctx.VoteInfos`
* [#17733](https://github.com/cosmos/cosmos-sdk/pull/17733) Ensure `buf export` exports all proto dependencies
* [#18063](https://github.com/cosmos/cosmos-sdk/pull/18063) Include additional information in the Info struct. This change enhances the Info struct by adding support for additional information through the ExtraInfo field
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

### Bug Fixes

Expand Down
10 changes: 10 additions & 0 deletions simapp/simd/cmd/commands.go
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"context"
"errors"
"io"
"os"
Expand All @@ -26,6 +27,7 @@ import (
servertypes "github.com/cosmos/cosmos-sdk/server/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/version"
authcmd "github.com/cosmos/cosmos-sdk/x/auth/client/cli"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
"github.com/cosmos/cosmos-sdk/x/crisis"
Expand All @@ -38,10 +40,18 @@ func initRootCmd(
interfaceRegistry codectypes.InterfaceRegistry,
appCodec codec.Codec,
basicManager module.BasicManager,
extraInfo version.ExtraInfo,
) {
cfg := sdk.GetConfig()
cfg.Seal()

type contextKey struct {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
samricotta marked this conversation as resolved.
Show resolved Hide resolved
key string
}

cmdContext := context.WithValue(rootCmd.Context(), version.ContextKey{}, extraInfo)
rootCmd.SetContext(cmdContext)

rootCmd.AddCommand(
genutilcli.InitCmd(basicManager),
NewTestnetCmd(basicManager, banktypes.GenesisBalancesIterator{}),
Expand Down
2 changes: 1 addition & 1 deletion simapp/simd/cmd/root_v2.go
Copy link
Member

Choose a reason for hiding this comment

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

you are missing an update in root.go, that's why the build is failing

Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func NewRootCmd() *cobra.Command {
},
}

initRootCmd(rootCmd, clientCtx.TxConfig, clientCtx.InterfaceRegistry, clientCtx.Codec, moduleBasicManager)
initRootCmd(rootCmd, clientCtx.TxConfig, clientCtx.InterfaceRegistry, clientCtx.Codec, moduleBasicManager, nil)

if err := autoCliOpts.EnhanceRootCommand(rootCmd); err != nil {
panic(err)
Expand Down
14 changes: 14 additions & 0 deletions version/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ func NewVersionCommand() *cobra.Command {
return nil
}

extraInfo := extraInfoFromContext(cmd)
verInfo.ExtraInfo = &extraInfo

var (
bz []byte
err error
Expand Down Expand Up @@ -56,3 +59,14 @@ func NewVersionCommand() *cobra.Command {

return cmd
}

func extraInfoFromContext(cmd *cobra.Command) ExtraInfo {
ctx := cmd.Context()
if ctx != nil {
extraInfo, ok := ctx.Value(ContextKey{}).(ExtraInfo)
if ok {
return extraInfo
}
}
return nil
}
samricotta marked this conversation as resolved.
Show resolved Hide resolved
7 changes: 7 additions & 0 deletions version/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import (
"runtime/debug"
)

// ContextKey is used to store the ExtraInfo in the context.
type ContextKey struct{}

var (
// application's name
Name = ""
Expand Down Expand Up @@ -55,6 +58,9 @@ func getSDKVersion() string {
return sdkVersion
}

// ExtraInfo contains a set of extra information provided by apps
type ExtraInfo map[string]string

Comment on lines +61 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

The ExtraInfo type is defined as a map of string to string. This is a flexible structure that can hold various types of information. However, it might be worth considering if a more structured approach would be beneficial, especially if there are specific pieces of information that will always be included in the ExtraInfo. If that's the case, a struct with specific fields might be a better choice.

// Info defines the application version information.
type Info struct {
Name string `json:"name" yaml:"name"`
Expand All @@ -65,6 +71,7 @@ type Info struct {
GoVersion string `json:"go" yaml:"go"`
BuildDeps []buildDep `json:"build_deps" yaml:"build_deps"`
CosmosSdkVersion string `json:"cosmos_sdk_version" yaml:"cosmos_sdk_version"`
ExtraInfo *ExtraInfo `json:"extra_info" yaml:"extra_info"`
}

func NewInfo() Info {
Expand Down
15 changes: 15 additions & 0 deletions version/version_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package version_test

import (
context "context"
Copy link
Contributor

Choose a reason for hiding this comment

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

The import of the context package is redundant as it's already included in the standard library and doesn't need to be aliased. You can directly use context.Background() or context.WithValue() without importing it explicitly.

- context "context"

Commitable suggestion (Beta)
Suggested change
context "context"
# context package is already included in the standard library
# You can directly use context.Background() or context.WithValue() without importing it explicitly.

"encoding/json"
"fmt"
"runtime"
Expand Down Expand Up @@ -158,7 +159,21 @@ func Test_runVersionCmd(t *testing.T) {

info := version.NewInfo()
stringInfo, err := json.Marshal(info)

extraInfo := &version.ExtraInfo{"key1": "value1"}
ctx := context.WithValue(context.Background(), version.ContextKey{}, extraInfo)
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

require.NoError(t, err)
require.NoError(t, cmd.Execute())

extraInfoFromContext := ctx.Value(version.ContextKey{})
assert.NotNil(t, extraInfoFromContext)

castedExtraInfo, ok := extraInfoFromContext.(*version.ExtraInfo)
assert.True(t, ok)

key1Value := (*castedExtraInfo)["key1"]
assert.Equal(t, "value1", key1Value)
samricotta marked this conversation as resolved.
Show resolved Hide resolved

assert.Equal(t, string(stringInfo)+"\n", mockOut.String())
}
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
Loading