Skip to content

Conversation

@mmorga
Copy link
Contributor

@mmorga mmorga commented Jul 12, 2016

This addresses issue #101

For each impacted Makefile, references to $(OCSIGENGROUP) have been replaced with “$(OCSIGENGROUP)”.

This addresses issue ocsigen#101

For each impacted Makefile, references to $(OCSIGENGROUP) have been replaced with “$(OCSIGENGROUP)”.
@vouillon
Copy link
Member

Can you really have a whitespace in a group name?

On my machine, man groupadd indicates:

       On Debian, the only constraints are that groupnames must neither start
       with a dash ('-') nor plus ('+') nor tilde ('~') nor contain a colon
       (':'), a comma (','), or a whitespace (space:' ', end of line: '\n',
       tabulation: '\t', etc.).

@vasilisp
Copy link
Contributor

vasilisp commented Jul 12, 2016

That does look weird. Are we talking about Cygwin or one of the other Unix-on-Windows layers?

In any case, the fix looks harmless, so we can merge it. Style-wise, I think we should push the quotes outside the whole string, e.g., "$(OCSIGENUSER):$(OCSIGENGROUP)". Maybe also apply the fix to all occurrences of $(OCSIGENUSER)? Why would the rules for user names be any more restrictive?

@vouillon vouillon merged commit e61fc0e into ocsigen:master Aug 3, 2016
@mmorga mmorga deleted the fix-group-makefiles branch March 7, 2017 05:13
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.

3 participants