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

gitolite integration doc should mention issue with empty-valued options #118

Closed
pfalcon opened this issue Aug 21, 2015 · 5 comments
Closed

Comments

@pfalcon
Copy link

pfalcon commented Aug 21, 2015

We're using git-multimail with gitolite and experiencing few problems. As I'm trying to investigate them further, I'd like to start recording them (as again, there're few (>1) issues). Worst case, it's kind of user error on our side. But even then, it may be worth to mention such issues on git-multmail docs.

So, git-multimail depends on being able to set some options to empty string (e.g. multimailhook.mailingList, and other multimailhook.*List options). We used obvious gitolite.conf syntax:

config multimailhook.commitList = ""

and only later spotted that with the syntax above, gitolite simply doesn't put multimailhook.commitList options to a particular repo's .git/config at all, which of course causes it behave rather different from the expectation.

@pfalcon
Copy link
Author

pfalcon commented Aug 21, 2015

Ok, trying various ways:

config multimailhook.commitList = ""

No option in .git/config

config multimailhook.commitList =

No option in .git/config

config multimailhook.commitList = " "

.git/config content:

[multimailhook]
    commitList = " "

Now only need to figure out how git-multimail would treat a single-space value for that (those!) options... And I hope you do agree all this stuff isn't logical at all, and worth being documented.

@pfalcon
Copy link
Author

pfalcon commented Aug 21, 2015

Now only need to figure out how git-multimail would treat a single-space value for that (those!) options...

.strip() in https://github.com/git-multimail/git-multimail/blob/master/git-multimail/git_multimail.py#L526 gives hope. But still, it's all too magic. @moy , would appreciate your commentary how to deal with this issue.

@moy
Copy link
Contributor

moy commented Aug 21, 2015

This is the same as #39 actually. Honestly, I consider it as a gitolite bug, but it's the documented behavior that '' acts as "unset", and I don't thing we can get that to change.

Yes, a single space is the way to go. I'll document it. Again, if you have better idea, reopen or submit a pull-request.

Thanks for the report.

@pfalcon
Copy link
Author

pfalcon commented Aug 24, 2015

Thanks for suggestion, pointer to #39, and applying doc fix!

Honestly, I consider it as a gitolite bug

Don't let me start to rant on Perl software (which gitolite is). We previously reported an issue when a single malformed SSH user key fed to gitolite broke processing of all remaining users after it. The response we got was along the lines of "even if it's small issue which affects a single user, making it like a nuke drop is helpful - then it won't be missed". Appeals that before it "won't be missed", there may still hours pass, with hundreds or thousands users being denied service, and such feature is outright denial-of-service mine (because a user who knows about it can use it deliberately to deny service to other users) fall on deaf ears. That's what happens when software is implemented in language which standardly didn't have exception handling (and I checked that Perl now has it - but nobody knows, uses, or even cares about it).

Anyway, as point, it's documented gitolite behavior, so reporting it upstream certainly doesn't have a sense, and working it around on git-multimail level what does.

Again, if you have better idea, reopen or submit a pull-request.

Well, few options in git-multimail already accept value of "none". It used even with string-valued options like multimailhook.replyTo (vs just enum-values options like multimailhook.smtpEncryption). So, would be logical to allow the same for commitList and friends (whereas a single space isn't logical, and one hardly could come up to it without reading docs or investigating issue).

Let me know if that sounds good, and I'll prepare a patch.

@moy
Copy link
Contributor

moy commented Aug 24, 2015

Well, few options in git-multimail already accept value of "none" [...] would be logical to allow the same for commitList and friends.

Sounds like a good idea, yes.

edward-dauvergne pushed a commit to edward-dauvergne/git-multimail that referenced this issue Aug 31, 2015
Fixes git-multimail#118.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
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

No branches or pull requests

2 participants