Open rubenseyer opened 6 years ago
I don't mind whether you keep the JSON support or not. If there is no use-case for it we could just drop it.
Sorry for leaving this be.
I'll quickly sketch up my thoughts on the personalities in Grumble:
I think the code we're working with now should just be "grumble". If we do implement personalities, they'd probably be a separate command (separate package main). That seems cleaner to me. The way I imagine it would pan out is that we refactor the Server part into a package, with the hooks necessary to implement custom handling.
Anyway, in my mind, since Murmur doesn't have config files at the moment, we should probably just use the Murmur .ini format all the way through.
I guess that does make some sense: Grumble, as it is right now has no way to set config options. (I removed my JSON-RPC thing and/or SSH thing I had for a while). With that in mind, I don't think there's much to lose from changing the config names Grumble uses already. Users of the current builds have no way to set them, right? Also, I don't think anyone really uses it in production, though I could be wrong, of course. So, there's no shame in dropping backwards compatibility with the freeze files.
Sorry for being inactive. I've been busy with other stuff and the ACME spec doesn't seem to be making a lot of progress anyway. I had some patches on my computer waiting for consensus that I am uploading now.
Seems like I am outvoted on the CamelCase point so I've dropped the compatibility layer design entirely and made grumble.ini configuration a superset of murmur.ini. (I still left some convenience features enabled in the .ini parser regarding quote escaping and boolean keys etc.).
I did keep the warnings for unsupported common Murmur configuration around but they are separate and easily removed now if you don't want them.
The branch history is a bit long now but I want to avoid rewriting the commits with issue mentions until the squash and merge.
Is @mkrautz waiting for changes by @rubenseyer here, still? I only mean well. Looks like this PR could solve a number issues related to not having a proper configuration file system in place!
Sorry to ask, but is there any news on this? I am about to setup a mumble server again, and would really like to use Grumble.
Given this PR is a few years out of date, if you would like to rebase it on master and give it a test that would be awesome. If I have time this weekend I can take a look myself.
If I have time this weekend I can take a look myself.
Would be awesome if you could have a look at it. Anything basic will do, if some channels could be preconfigured like in uMurmur that's totally sufficient.
Wow, I had forgotten about this! I'm still around, but probably too busy to do anything too soon. I'll try to rebase and update everything if I get time and no one else beats me.
Looking through what has happened since I'm gone, here's a few things off the top of my head:
Sorry for dropping a book on you, but I thought I'd write down some of these considerations so we can think about the decisions involved. Merging this is not far away in terms of code changes but it is an entirely new part to Grumble so it was always gonna be a matter of design problems first and foremost.
It sounds like this PR brings up a bunch of questions that we should figure out the best way to handle.
I think my views on this have remained unchanged since I first began this PR --- in essence, I agree.
I think that for Grumble to become a default choice we will need compatibility to allow easy migration (but compatibility need not mean that we enforce Grumble to have exactly the same feature set as Murmur). My solution for that was the translation layer which was a compromise between not breaking existing Grumble servers and allowing compatibility with existing configuration, because I have no problem with the real Grumble config not being exactly the same as murmur.ini (and apparently neither did the original author of serverconf
) but I understand it seems like a lot of trouble just because we don't want to rename config keys.
(Also, we clearly would need gRPC for any major production user to consider switching. That was my idea for the next step after this PR back in 2018.)
I rebased everything and re-organized the slew of WIP commits into something more logical. I have changed no design decisions here compared to where I left off in 2018, so I do not expect this to be merged until everyone is satisfied on those. Please do test and review the changes, because I have only built it and nothing more.
Some notes:
I get this error on first start. The message is also missing a "\n".
Unable to open log file (/home/mumble/.grumble/grumble.log): open : no such file or directory
I'm not able to add @all
permissions to the root channel, it only adds another SuperUser
everytime I try. Is this a new problem with this PR or a known issue? I also cannot start the server anymore. Could this be related?
$ ./grumble
[G] 2020/06/10 16:55:12.632307 Grumble
[G] 2020/06/10 16:55:12.633083 Using data directory: /home/mumble/.grumble
[G] 2020/06/10 16:55:12.633692 Loading server 1
panic: assignment to entry in nil map
goroutine 1 [running]:
main.(*Channel).Unfreeze(0xc0000f4360, 0xc0000f4120)
/home/mumble/repos/grumble/cmd/grumble/freeze.go:272 +0x4d2
main.NewServerFromFrozen(0xd015f1, 0x1, 0xc0000bec80, 0x0, 0x0)
/home/mumble/repos/grumble/cmd/grumble/freeze.go:451 +0x808
main.main()
/home/mumble/repos/grumble/cmd/grumble/grumble.go:245 +0x1494
I can reproduce the same crash on both master and this PR:
@all
aclI have filed PR #71 to fix it, because it is not really pertinent to this one (which is bogged down in years of reviews anyway). You should be able to easily rebase those changes onto this one if you want to combine them immediately -- this shouldn't touch those areas too much. I'll rebase here later if the other one is merged.
Alright, so here's my suggestion for what the config system could look like. Sorry for being a bit spammy with the issue mentions--I didn't expect them to show up while I was still tidying history. I'm holding off on the Let's Encrypt implementation because I want to see how the
tls-sni
debacle ends before adding another HTTP server just to respond to the challenge.serverconf
based on string maps. It is format independent - if it can encode string key-value pairs (and has a file format extension) it can be a config format.If the config file is named murmur.ini an additional translation step is performed, including warnings for some unsupported features (but I won't promise I catch all of them). (The .ini support uses go-ini.)but I quickly implemented it anyway to showcase the format independence. However, it would probably require a few good test cases for me to be confident it's usable.The freeze system still acts as the persistent storage, which means that to revert a config value to defaults, you can't just comment it out, you have to specify it unset. I think this is an acceptable compromise.The config system introduces one new feature: if the config format supports some form of subsets (sections in .ini, nested objects in .json) it is possible to set specific overrides for single virtual servers (including reverting to defaults). Not the best form of virtual server management, but at least it's something.To make this meaningful, I also implemented some missing config keys that users migrating from Murmur probably would want. Features like databases, RPC and bandwidth limiting are still missing (but the latter should probably wait for web client support). I also added the SuperUser CLI flags from Murmur. I think the remaining unimplemented Murmur CLI flags would be meaningless for Grumble right now, at least.