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

common: fix #1818, secondary datadir paths to fall back to #1856

Merged
merged 1 commit into from
Oct 1, 2015

Conversation

karalabe
Copy link
Member

This PR fixes an issue in Fedora and Android where the current user cannot be retrieved on certain platforms (Actually, the fix is more generic). If the user cannot be accessed, the default data dir uses a workspace in the current working folder. If that also cannot be retrieved, it defaults to an empty folder, leaving it to the user to sort it out manually via CLI flags (still better than crashing).

@robotally
Copy link

Vote Count Reviewers
👍 1 @Gustav-Simonsson
👎 0
Reaction Users
👍 @0mkara

Updated: Thu Oct 1 13:30:59 UTC 2015

@karalabe
Copy link
Member Author

Hey @0mkara, if you have the time, could you check if this fix solves the homedir issue for you on Fedora? Thanks!

return filepath.Join(dir, "Ethereum")
} else if runtime.GOOS == "windows" {
return filepath.Join(dir, "Ethereum")
} else {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this. On Android this will probably be fine. On Fedora it is a bug and when it gets fixed the datadir might change. And each time the user starts geth in a different dir the datadir is different. This can get really confusing. Maybe its best on Fedora to exit the program and print a message informing the user that he needs to specify the datadir manually.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, another option would be to drop the second block (the pwd one), and check in geth main that an empty data dir should return an error. I'm fine with that approach too. This is actually why I never implemented this, because I couldn't decide what the best course of action is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using os.Getenv("HOME") as a fallback.
That should also work on Fedora in most cases.

@0mkara
Copy link

0mkara commented Sep 29, 2015

Yes @karalabe this worked on fedora for me.

@karalabe karalabe force-pushed the andorid-path-fix branch 2 times, most recently from 77ed191 to 74578ab Compare October 1, 2015 09:26
@Gustav-Simonsson
Copy link

👍

obscuren added a commit that referenced this pull request Oct 1, 2015
common: fix #1818, secondary datadir paths to fall back to
@obscuren obscuren merged commit 581c090 into ethereum:develop Oct 1, 2015
@obscuren obscuren removed the review label Oct 1, 2015
@0mkara
Copy link

0mkara commented Oct 1, 2015

👍

maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Nov 1, 2023
…merge_v1.10.16_v1.12.2

Merge develop to big merge v1.10.16 v1.12.2
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.

7 participants