Re: [PATCH] chroot specify user/group feature

[Giuseppe Scrivano]

Jim Meyering <address@hidden> writes:

> In the future, please send patches by mail to the bug-coreutils list.
> Most of the people who will review them here prefer that.

Sorry, I thought it was redundant to post both on the ML and on
savannah.  I'll send the next version to the bug-coreutils ML.

> If you'd like to pursue, here are some suggestions:
>
>   - MAXUID was defined but never used, so please remove it
>   - there is a --help formatting nit exposed by failing "make syntax-check":
>
> src/chroot.c:122:  -u, --userspec specify the userspec in the form 
> USER:GROUP\n\
> maint.mk: help2man requires at least two spaces between
> maint.mk: an option and its description

Thank you, I didn't know about it.


> make: *** [sc_two_space_separator_in_usage] Error 1
>
>   - do not add short-named options.  People who want to
>       abbreviate can use --u or --g.
>   - exit right away upon encountering an invalid option.
>       Make the option-processing loop look like most others, i.e.,
>       with a switch and anonymous enum values in place of your
>       current 'u' and 'g'.

What do you think if I change 'g' to 'G'?  I looked under FreeBSD and
they use -u, -g and -G, maybe it worths to keep the same name.
Under OpenBSD it is not possible to specify additional groups, so only
-u and -g are available.


>   - add a NEWS entry
>   - add documentation in coreutils.texi
>
> And for extra credit,
>
>   - add a test or two to exercise the new functionality.
>       It'll have to be a root-only test, of course.

Sure, I thought about add all these things after you approved it.

Regards,
Giuseppe