xorpaul / g10k

my r10k fork in Go
Apache License 2.0
125 stars 50 forks source link

Check writability of configured CacheDir. #135

Closed pillarsdotnet closed 5 years ago

pillarsdotnet commented 5 years ago

Fixes: #136

I was getting some confusing segfaults when the ownership of the g10k CacheDir was wrong.

The program should print an error instead of segfaulting. I'm hoping this change does the trick, but I've never written in Golang before.

pillarsdotnet commented 5 years ago

Compiled and tested locally; works.

pillarsdotnet commented 5 years ago

I'm trying to figure out how to write a test. Looks like I need to modify g10k_test.go and add a file to the tests directory.

xorpaul commented 5 years ago

I'm trying to figure out how to write a test. Looks like I need to modify g10k_test.go and add a file to the tests directory.

Correct, but you don't need to create a dedicates g10k config file in the tests directory.

You could just use any of the already existing g10k config files and create the cachedir and remove write permission before calling the resolvePuppetEnvironment function like in the other test function.

Even without this test I would still merge this and add the test later by myself, because it's not that easy to understand how I setup the g10k tests and I'm fully aware that this is my fault and there are definitely easier ways to write tests. :smirk:

Or I could leave this open if you're interested in writing the g10k test yourself? Your call :smiley:

pillarsdotnet commented 5 years ago

@xorpaul -- Okay; I give up. Setting perms to 0555 doesn't seem to prevent writing. Removing the test.

xorpaul commented 5 years ago

No problem, I'm going to merge this and add the test later.

I'll send you a link to test if you're interested :smirk:

xorpaul commented 5 years ago

There you go: https://github.com/xorpaul/g10k/commit/3af6e209bc811f3793b14eb4c8f827995b6a2c93

Thanks again!