Skip to content

cmd/geth: fix passing datadir flag to attach subcommand#15517

Merged
fjl merged 1 commit into
ethereum:masterfrom
MaximilianMeister:usage-ipcpath
Nov 28, 2017
Merged

cmd/geth: fix passing datadir flag to attach subcommand#15517
fjl merged 1 commit into
ethereum:masterfrom
MaximilianMeister:usage-ipcpath

Conversation

@MaximilianMeister
Copy link
Copy Markdown
Contributor

@MaximilianMeister MaximilianMeister commented Nov 18, 2017

the datadir flag wasn't respected, instead you had to provide an
additional argument, which was unclear from a usage point of view

Signed-off-by: Maximilian Meister mmeister@suse.de

I tried the following on a private network:

geth --datadir /etc/testnet/miner attach
Fatal: Unable to attach to remote geth: dial unix /root/.ethereum/geth.ipc: connect: no such file or directory
command terminated with exit code 1

Comment thread cmd/geth/consolecmd.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could remove the append here.

@karalabe
Copy link
Copy Markdown
Member

Perhaps we should fix the subcommand to accept --datadir instead?

@MaximilianMeister
Copy link
Copy Markdown
Contributor Author

MaximilianMeister commented Nov 23, 2017

Perhaps we should fix the subcommand to accept --datadir instead?

@karalabe is this the right way to fix it? i just assumed that the ipc endpoint will always be called geth.ipc
not 100% sure if that's the case, i just guess so because there is no --ipcpath option for attach to override it

@MaximilianMeister MaximilianMeister changed the title cmd: clarify usage of the attach subcommand cmd/geth: fix passing datadir flag to attach subcommand Nov 23, 2017
Copy link
Copy Markdown

@lion22 lion22 left a comment

Choose a reason for hiding this comment

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

0

Comment thread cmd/geth/consolecmd.go Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

 конечная точка  : = ctx. Args (). Первый ()
  • если ctx. GlobalIsSet (utils. DataDirFlag . Name ) {
  • endpoint = fmt. Sprintf ( " % s /geth.ipc " , ctx. GlobalString (utils. DataDirFlag . Name ))
    +}
  • клиент , err : = dialRPC (конечная точка)

Comment thread cmd/geth/consolecmd.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is fine, but please use --datadir only if the first argument is not set.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@fjl done, thanks

Comment thread cmd/geth/consolecmd.go Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please remove unnecessary parens here. You can write endpoint == "" && ctx.GlobalIsSet(utils.DataDirFlag.Name).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@fjl sure, fixed

the datadir flag wasn't respected, instead you had to provide an
additional argument, which was unclear from a usage point of view

Signed-off-by: Maximilian Meister <mmeister@suse.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants