-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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: parse home flag earlier #20771
Changes from all commits
0acd9a9
c8cbee9
743f1d3
a7ebdae
0f05e65
1b53ff4
86900bd
3b3192a
8b83783
040b46c
dddddaf
022bb75
f98798f
e8d3ca1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
package helpers | ||
|
||
import ( | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
) | ||
|
||
// GetNodeHomeDirectory gets the home directory of the node (where the config is located). | ||
// It parses the home flag if set if the `NODE_HOME` environment variable if set (and ignores name). | ||
// Otherwise, it returns the default home directory given its name. | ||
func GetNodeHomeDirectory(name string) (string, error) { | ||
// get the home directory from the flag | ||
args := os.Args | ||
for i := 0; i < len(args); i++ { | ||
if args[i] == "--home" && i+1 < len(args) { | ||
return filepath.Clean(args[i+1]), nil | ||
} else if strings.HasPrefix(args[i], "--home=") { | ||
return filepath.Clean(args[i][7:]), nil | ||
} | ||
} | ||
|
||
// get the home directory from the environment variable | ||
homeDir := os.Getenv("NODE_HOME") | ||
if homeDir != "" { | ||
return filepath.Clean(homeDir), nil | ||
} | ||
|
||
// return the default home directory | ||
userHomeDir, err := os.UserHomeDir() | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
return filepath.Join(userHomeDir, name), nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,7 @@ SIMD_BIN=${SIMD_BIN:=$(which simd 2>/dev/null)} | |
|
||
if [ -z "$SIMD_BIN" ]; then echo "SIMD_BIN is not set. Make sure to run make install before"; exit 1; fi | ||
echo "using $SIMD_BIN" | ||
if [ -d "$($SIMD_BIN config home)" ]; then rm -r $($SIMD_BIN config home); fi | ||
if [ -d "$($SIMD_BIN config home)" ]; then rm -rv $($SIMD_BIN config home); fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quote the command substitution to prevent word splitting. The script uses command substitution without quoting, which can lead to word splitting and unexpected behavior, especially when directory names contain spaces. - if [ -d "$($SIMD_BIN config home)" ]; then rm -rv $($SIMD_BIN config home); fi
+ if [ -d "$("$SIMD_BIN" config home)" ]; then rm -rv "$("$SIMD_BIN" config home)"; fi ToolsShellcheck
|
||
$SIMD_BIN config set client chain-id demo | ||
$SIMD_BIN config set client keyring-backend test | ||
$SIMD_BIN config set client keyring-default-keyname alice | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,7 @@ CONFIG="${CONFIG:-$HOME/.simappv2/config}" | |
cd "$SIMAPP_DIR" | ||
go build -o "$ROOT/build/simdv2" simdv2/main.go | ||
|
||
if [ -d "$($SIMD config home)" ]; then rm -r $($SIMD config home); fi | ||
if [ -d "$($SIMD config home)" ]; then rm -rv $($SIMD config home); fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quote the command substitution to prevent word splitting. As with the previous script, this line suffers from the same issue where command substitution could lead to word splitting. It's a good practice to quote these to ensure the command behaves as expected. - if [ -d "$($SIMD config home)" ]; then rm -rv $($SIMD config home); fi
+ if [ -d "$("$SIMD" config home)" ]; then rm -rv "$("$SIMD" config home)"; fi ToolsShellcheck
|
||
|
||
$SIMD init simapp-v2-node --chain-id simapp-v2-chain | ||
|
||
|
@@ -28,6 +28,7 @@ jq '.app_state.mint.minter.inflation = "0.300000000000000000"' genesis.json > te | |
# change the initial height to 2 to work around store/v2 and iavl limitations with a genesis block | ||
jq '.initial_height = 2' genesis.json > temp.json && mv temp.json genesis.json | ||
|
||
$SIMD config set client chain-id simapp-v2-chain | ||
$SIMD keys add test_validator --indiscreet | ||
VALIDATOR_ADDRESS=$($SIMD keys show test_validator -a --keyring-backend test) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import ( | |
autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" | ||
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1" | ||
"cosmossdk.io/client/v2/autocli" | ||
clienthelpers "cosmossdk.io/client/v2/helpers" | ||
"cosmossdk.io/core/log" | ||
storetypes "cosmossdk.io/store/types" | ||
"cosmossdk.io/x/accounts" | ||
|
@@ -183,12 +184,11 @@ type SimApp struct { | |
} | ||
|
||
func init() { | ||
userHomeDir, err := os.UserHomeDir() | ||
var err error | ||
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp") | ||
Comment on lines
+187
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Review the initialization of The modification to initialize However, the use of - panic(err)
+ log.Fatalf("Failed to set DefaultNodeHome: %v", err)
|
||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
DefaultNodeHome = filepath.Join(userHomeDir, ".simapp") | ||
} | ||
|
||
// NewSimApp returns a reference to an initialized SimApp. | ||
|
@@ -536,7 +536,7 @@ func NewSimApp( | |
app.sm = module.NewSimulationManagerFromAppModules(app.ModuleManager.Modules, overrideModules) | ||
|
||
// create, start, and load the unordered tx manager | ||
utxDataDir := filepath.Join(cast.ToString(appOpts.Get(flags.FlagHome)), "data") | ||
utxDataDir := filepath.Join(homePath, "data") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tip Codebase Verification Ensure the directory The directory creation or existence check for
Suggested Code: utxDataDir := filepath.Join(homePath, "data")
if err := os.MkdirAll(utxDataDir, 0755); err != nil {
log.Fatalf("failed to create directory %s: %v", utxDataDir, err)
}
app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir) Please review and incorporate this change to ensure the directory is available. Analysis chainReview the setup of the Unordered Transaction Manager. The setup of the However, ensure that the directory Sub-tasksSub-task 1: Verify if the directory creation logic exists in the
|
||
app.UnorderedTxManager = unorderedtx.NewManager(utxDataDir) | ||
app.UnorderedTxManager.Start() | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -6,12 +6,12 @@ import ( | |||||||||||||||||||
_ "embed" | ||||||||||||||||||||
"fmt" | ||||||||||||||||||||
"io" | ||||||||||||||||||||
"os" | ||||||||||||||||||||
"path/filepath" | ||||||||||||||||||||
|
||||||||||||||||||||
dbm "github.com/cosmos/cosmos-db" | ||||||||||||||||||||
"github.com/spf13/cast" | ||||||||||||||||||||
|
||||||||||||||||||||
clienthelpers "cosmossdk.io/client/v2/helpers" | ||||||||||||||||||||
"cosmossdk.io/core/legacy" | ||||||||||||||||||||
"cosmossdk.io/depinject" | ||||||||||||||||||||
"cosmossdk.io/log" | ||||||||||||||||||||
|
@@ -100,12 +100,11 @@ type SimApp struct { | |||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func init() { | ||||||||||||||||||||
userHomeDir, err := os.UserHomeDir() | ||||||||||||||||||||
var err error | ||||||||||||||||||||
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp") | ||||||||||||||||||||
Comment on lines
+103
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use consistent error handling across initializers. Similar to the previous file, this func init() {
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simapp")
- if err != nil {
- panic(err)
+ if err != nil {
+ log.Fatalf("Failed to get node home directory: %v", err)
}
} Committable suggestion
Suggested change
|
||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
panic(err) | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
DefaultNodeHome = filepath.Join(userHomeDir, ".simapp") | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
// AppConfig returns the default app config. | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,7 +30,7 @@ import ( | |
func NewRootCmd() *cobra.Command { | ||
// we "pre"-instantiate the application for getting the injected/configured encoding configuration | ||
// note, this is not necessary when using app wiring, as depinject can be directly used (see root_v2.go) | ||
tempApp := simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(tempDir())) | ||
tempApp := simapp.NewSimApp(log.NewNopLogger(), dbm.NewMemDB(), nil, true, simtestutil.NewAppOptionsWithFlagHome(simapp.DefaultNodeHome)) | ||
encodingConfig := params.EncodingConfig{ | ||
InterfaceRegistry: tempApp.InterfaceRegistry(), | ||
Codec: tempApp.AppCodec(), | ||
|
@@ -114,7 +114,7 @@ func NewRootCmd() *cobra.Command { | |
// autocli opts | ||
customClientTemplate, customClientConfig := initClientConfig() | ||
var err error | ||
initClientCtx, err = config.ReadDefaultValuesFromDefaultClientConfig(initClientCtx, customClientTemplate, customClientConfig) | ||
initClientCtx, err = config.CreateClientConfig(initClientCtx, customClientTemplate, customClientConfig) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor error handling in configuration setup. Consistent with previous files, consider returning errors instead of using - panic(err)
+ return nil, err
|
||
if err != nil { | ||
panic(err) | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,7 +24,6 @@ import ( | |||||||||||
"github.com/cosmos/cosmos-sdk/codec" | ||||||||||||
codectypes "github.com/cosmos/cosmos-sdk/codec/types" | ||||||||||||
"github.com/cosmos/cosmos-sdk/server" | ||||||||||||
simtestutil "github.com/cosmos/cosmos-sdk/testutil/sims" | ||||||||||||
"github.com/cosmos/cosmos-sdk/types/module" | ||||||||||||
) | ||||||||||||
|
||||||||||||
|
@@ -38,10 +37,7 @@ func NewRootCmd() *cobra.Command { | |||||||||||
|
||||||||||||
if err := depinject.Inject( | ||||||||||||
depinject.Configs(simapp.AppConfig(), | ||||||||||||
depinject.Supply( | ||||||||||||
log.NewNopLogger(), | ||||||||||||
simtestutil.NewAppOptionsWithFlagHome(tempDir()), | ||||||||||||
), | ||||||||||||
depinject.Supply(log.NewNopLogger()), | ||||||||||||
depinject.Provide( | ||||||||||||
ProvideClientContext, | ||||||||||||
), | ||||||||||||
|
@@ -128,7 +124,7 @@ func ProvideClientContext( | |||||||||||
|
||||||||||||
// Read the config to overwrite the default values with the values from the config file | ||||||||||||
customClientTemplate, customClientConfig := initClientConfig() | ||||||||||||
clientCtx, err = config.ReadDefaultValuesFromDefaultClientConfig(clientCtx, customClientTemplate, customClientConfig) | ||||||||||||
clientCtx, err = config.CreateClientConfig(clientCtx, customClientTemplate, customClientConfig) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Refactor error handling in Similar to the previous file, consider improving the error handling by returning errors instead of using - panic(err)
+ return client.Context{}, err Committable suggestion
Suggested change
|
||||||||||||
if err != nil { | ||||||||||||
panic(err) | ||||||||||||
} | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,16 +2,17 @@ package simapp | |||||||||||||||||||
|
||||||||||||||||||||
import ( | ||||||||||||||||||||
_ "embed" | ||||||||||||||||||||
"os" | ||||||||||||||||||||
"path/filepath" | ||||||||||||||||||||
|
||||||||||||||||||||
"github.com/spf13/viper" | ||||||||||||||||||||
|
||||||||||||||||||||
clienthelpers "cosmossdk.io/client/v2/helpers" | ||||||||||||||||||||
coreapp "cosmossdk.io/core/app" | ||||||||||||||||||||
"cosmossdk.io/core/legacy" | ||||||||||||||||||||
"cosmossdk.io/core/log" | ||||||||||||||||||||
"cosmossdk.io/depinject" | ||||||||||||||||||||
"cosmossdk.io/runtime/v2" | ||||||||||||||||||||
serverv2 "cosmossdk.io/server/v2" | ||||||||||||||||||||
"cosmossdk.io/store/v2" | ||||||||||||||||||||
"cosmossdk.io/store/v2/commitment/iavl" | ||||||||||||||||||||
"cosmossdk.io/store/v2/db" | ||||||||||||||||||||
|
@@ -36,10 +37,8 @@ import ( | |||||||||||||||||||
upgradekeeper "cosmossdk.io/x/upgrade/keeper" | ||||||||||||||||||||
|
||||||||||||||||||||
"github.com/cosmos/cosmos-sdk/client" | ||||||||||||||||||||
"github.com/cosmos/cosmos-sdk/client/flags" | ||||||||||||||||||||
"github.com/cosmos/cosmos-sdk/codec" | ||||||||||||||||||||
codectypes "github.com/cosmos/cosmos-sdk/codec/types" | ||||||||||||||||||||
servertypes "github.com/cosmos/cosmos-sdk/server/types" | ||||||||||||||||||||
"github.com/cosmos/cosmos-sdk/std" | ||||||||||||||||||||
) | ||||||||||||||||||||
|
||||||||||||||||||||
|
@@ -77,12 +76,11 @@ type SimApp struct { | |||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
func init() { | ||||||||||||||||||||
userHomeDir, err := os.UserHomeDir() | ||||||||||||||||||||
var err error | ||||||||||||||||||||
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2") | ||||||||||||||||||||
Comment on lines
+79
to
+80
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Proper error handling in initialization. The func init() {
var err error
DefaultNodeHome, err = clienthelpers.GetNodeHomeDirectory(".simappv2")
- if err != nil {
- panic(err)
+ if err != nil {
+ log.Fatalf("Failed to get node home directory: %v", err)
}
} Committable suggestion
Suggested change
|
||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
panic(err) | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
DefaultNodeHome = filepath.Join(userHomeDir, ".simappv2") | ||||||||||||||||||||
} | ||||||||||||||||||||
|
||||||||||||||||||||
// AppConfig returns the default app config. | ||||||||||||||||||||
|
@@ -97,8 +95,8 @@ func NewSimApp( | |||||||||||||||||||
logger log.Logger, | ||||||||||||||||||||
viper *viper.Viper, | ||||||||||||||||||||
) *SimApp { | ||||||||||||||||||||
homeDir := viper.Get(flags.FlagHome).(string) // TODO | ||||||||||||||||||||
scRawDb, err := db.NewGoLevelDB("application", filepath.Join(homeDir, "data"), nil) | ||||||||||||||||||||
viper.Set(serverv2.FlagHome, DefaultNodeHome) // TODO possibly set earlier when viper is created | ||||||||||||||||||||
scRawDb, err := db.NewGoLevelDB("application", filepath.Join(DefaultNodeHome, "data"), nil) | ||||||||||||||||||||
if err != nil { | ||||||||||||||||||||
panic(err) | ||||||||||||||||||||
} | ||||||||||||||||||||
|
@@ -113,7 +111,7 @@ func NewSimApp( | |||||||||||||||||||
logger, | ||||||||||||||||||||
&root.FactoryOptions{ | ||||||||||||||||||||
Logger: logger, | ||||||||||||||||||||
RootDir: homeDir, | ||||||||||||||||||||
RootDir: DefaultNodeHome, | ||||||||||||||||||||
SSType: 0, | ||||||||||||||||||||
SCType: 0, | ||||||||||||||||||||
SCPruneOptions: &store.PruneOptions{ | ||||||||||||||||||||
|
@@ -126,7 +124,7 @@ func NewSimApp( | |||||||||||||||||||
}, | ||||||||||||||||||||
SCRawDB: scRawDb, | ||||||||||||||||||||
}, | ||||||||||||||||||||
servertypes.AppOptions(viper), | ||||||||||||||||||||
viper, | ||||||||||||||||||||
|
||||||||||||||||||||
// ADVANCED CONFIGURATION | ||||||||||||||||||||
|
||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the changelog entry for clarity and accuracy.
The entry mentions the removal of
ReadDefaultValuesFromDefaultClientConfig
from theclient
package. Ensure that the entry clearly states that this removal is part of the broader changes for parsing the--home
flag earlier, which is the main focus of the PR.Committable suggestion