Removed call to `TryCreateDirectory` from `GetDefaultDataDir` in `src/util.cpp`. #7850

See #7845 (comment).

utACK. If there is a good reason for that line to be there on OSX only, then it needs a comment justifying it.

It seems that this was there as long as GetDefaultDatadir has been there: https://github.com/bitcoin/bitcoin/tree/d882773789ea3894de7163f7bb880c5b23072882/util.cpp

Looks like this is not for merging...

??

Sent from my iPhone

On 10 Apr 2016, at 16:40, paveljanik notifications@github.com wrote:

Looks like this is not for merging...

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

According to the comment mentioned above, you are only trying if this will work. Your real problem is different though, I think. Can you investigate it a bit more please?

pavel$ sudo -u test env | grep HOME
HOME=/Users/pavel
pavel$ sudo -H -u test env | grep HOME
HOME=/Users/test
pavel$ 

@mrCertified ?

@paveljanik The change is for merging. As I said above, if there is a reason for the line to be there it needs to be justified with a comment.

And I added that the behaviour has been there for as long as the GetDefaultDatadir function has existed, implying that we'll probably never find out why.

Concept ACK

@sipa yes, but someone should really investigate, how bitcoind/-Qt works when the parent directory (ie. Library/Application Support) is not created/is not writable by the current user or does not exist at all (where/how will we write the config file, blocks, chainstate etc?). I suspect this hides some other problem down in the code...

But if proven to work (not just compile ;-), I'm happy with making OS X the same as "Unix" branch there - @alexreg can you please make the OS X branch to directly return the full path and not changing it in two passes (1. add L/A S, 2. add /Bitcoin) and then test with a real user without that directory or with bad permissions on that directory? How will bitcoind/-Qt work in such cases?

But if the data directory has been changed to a different value, the only reason GetDefaultDataDir() is called is to display what the default would have been. (I'm ignoring the slightly more complicated behaviour in -qt). So making sure that (part of) that path exists makes no sense.

... exist and is writable...

utACK. Creating ~/Library/Application Support is useless (on OSX).

I’m not sure exactly what you mean. I don’t really code C++. Could you give me the diff please, and I’ll apply it?

I’ve already tested both those cases though, and it works as it should.

On 11 Apr 2016, at 08:30, paveljanik notifications@github.com wrote:

@sipa https://github.com/sipa yes, but someone should really investigate, how bitcoind/-Qt works when the parent directory (ie. Library/Application Support) is not created/is not writable by the current user or does not exist at all (where/how will we write the config file, blocks, chainstate etc?). I suspect this hides some other problem down in the code...

But if proven to work (not just compile ;-), I'm happy with making OS X the same as "Unix" branch there - @alexreg https://github.com/alexreg can you please make the OS X branch to directly return the full path and not changing it in two passes (1. add L/A S, 2. add /Bitcoin) and then test with a real user without that directory or with bad permissions on that directory? How will bitcoind/-Qt work in such cases?

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub #7850 (comment)

Just return pathRet / "Library/Application Support/Bitcoin" ...

Oh right. Done now.

utACK 0f2fd2cec05b90d071b30eb522061e1c5144b367 after squash

Please squash both commits into one.

I don’t use Git much, so no idea how to do that.

On 12 Apr 2016, at 21:00, paveljanik notifications@github.com wrote:

Please squash both commits into one.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub #7850 (comment)

Wladimir explains in #7458, or just do a google search

utACK https://github.com/bitcoin/bitcoin/pull/7850/commits/41dbc4849e0bf14eef98962b0f0bddcde0bb3014

ACK https://github.com/bitcoin/bitcoin/pull/7850/commits/41dbc4849e0bf14eef98962b0f0bddcde0bb3014

Hmm this is indeed weird, why would this need a TryCreateDirectory on OSX.

Happy that this solves your problem.

utACK https://github.com/bitcoin/bitcoin/pull/7850/commits/41dbc4849e0bf14eef98962b0f0bddcde0bb3014

I digged in the git history, but could not find something important. But here is the wild guess: on OS X, the config files are in ~/Library/Application Support. On other unixes, config directory .bitcoin is directly in the home directory. And the wild guess is that this "mkdir -p parent_directory" had to be somewhere, so someone decided it could be here 8)

Thanks for accepting this, guys.

Sent from my iPhone

On 14 Apr 2016, at 12:28, Wladimir J. van der Laan notifications@github.com wrote:

Hmm this is indeed weird, why would this need a TryCreateDirectory on OSX.

Happy that this solves your problem.

utACK 41dbc48

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

Backported as part of #7938. Removing label 'Needs backport'.