[rhel7/master/f21 2/2] Add support for custom gid to advanced user setup (#1163803)
Anne Mulhern
amulhern at redhat.com
Mon Nov 17 13:50:54 UTC 2014
----- Original Message -----
> From: "Brian C. Lane" <bcl at redhat.com>
> To: anaconda-patches at lists.fedorahosted.org
> Sent: Friday, November 14, 2014 9:00:43 PM
> Subject: [rhel7/master/f21 2/2] Add support for custom gid to advanced user setup (#1163803)
>
> The dialog for the advanced user setup claims you can specifiy the group
> id like this: groupname (1234), but support for that was never actually
> added.
>
> This adds support for parsing the gid, as well as tests for the new
> parse function. Note that if it cannot parse the group entry it will
> ignore it. At some future time we should rework error handling in this
> dialog to make this more user friendly.
>
> Resolves: rhbz#1163803
> ---
> pyanaconda/iutil.py | 26 +++++++++++++++++++++++++-
> pyanaconda/regexes.py | 7 +++++++
> pyanaconda/ui/gui/spokes/user.py | 10 +++++++---
> tests/pyanaconda_tests/iutil_test.py | 9 +++++++++
> 4 files changed, 48 insertions(+), 4 deletions(-)
>
> diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> index 7e3dde3..7d83dbe 100644
> --- a/pyanaconda/iutil.py
> +++ b/pyanaconda/iutil.py
> @@ -35,7 +35,7 @@ from Queue import Queue, Empty
> from pyanaconda.flags import flags
> from pyanaconda.constants import DRACUT_SHUTDOWN_EJECT, ROOT_PATH,
> TRANSLATIONS_UPDATE_DIR, UNSUPPORTED_HW
> from pyanaconda.constants import SCREENSHOTS_DIRECTORY,
> SCREENSHOTS_TARGET_DIRECTORY
> -from pyanaconda.regexes import PROXY_URL_PARSE
> +from pyanaconda.regexes import PROXY_URL_PARSE, GROUP_STR_PARSE,
> GROUPNAME_VALID
>
> import logging
> log = logging.getLogger("anaconda")
> @@ -887,3 +887,27 @@ def save_screenshots():
> def parent_dir(directory):
> """Return the parent directories"""
> return "/".join(os.path.normpath(directory).split("/")[:-1])
> +
> +def parse_group_str(group_str):
> + """Parse the group string for advanced user setup.
> +
> + :param str group_str: group string
> + :returns: tuple of group name and group id
> +
> + The string can be one of:
> + group
> + group (gid)
> + group(gid)
> +
> + If there is no gid it will return None for it.
> + If there is an error parsing it will return ("", None)
> + """
> + m = GROUP_STR_PARSE.match(group_str)
> + if not m or not GROUPNAME_VALID.match(m.group(1)):
> + return ("", None)
> +
> + gid = None
> + if m.group(3):
> + gid = int(m.group(3))
> +
> + return (m.group(1), gid)
Might be slightly prettier to do something like:
m = GROUP_STR_PARSE.match(group_str)
if not m:
return ("", None)
(group_name, group_id) = m.group(1,3)
if not GROUPNAME_VALID.match(group_name):
return ("", None)
return (group_name, group_id and int(group_id))
> diff --git a/pyanaconda/regexes.py b/pyanaconda/regexes.py
> index 595b26c..e1761dd 100644
> --- a/pyanaconda/regexes.py
> +++ b/pyanaconda/regexes.py
> @@ -56,3 +56,10 @@ GROUPLIST_SIMPLE_VALID = re.compile(r'^\s*(' +
> _USERNAME_BASE + r'(\s*,\s*' + _U
>
> # Proxy parsing
> PROXY_URL_PARSE =
> re.compile("([A-Za-z]+://)?(([A-Za-z0-9]+)(:[^:@]+)?@)?([^:/]+)(:[0-9]+)?(/.*)?")
> +
> +# Group string parsing
> +# It can be a group name by itself or a group name followed by a gid inside
> ()
> +# match.group(1) = group name
> +# match.group(2) = (gid)
> +# match.group(3) = gid
> +GROUP_STR_PARSE = re.compile(r'^\s*([a-zA-Z0-9._-]+)\s*(\((\d+)\))*')
The * on the end here means that you match things like:
group(GID1)(GID2), as:
>>> z = GROUP_STR_PARSE.match("group(123)(456)"
>>> z.group(0,1,2,3)
('group(123)(456)', 'group', '(456)', '456')
? is probably better than *.
?P syntax might be useful here.
> diff --git a/pyanaconda/ui/gui/spokes/user.py
> b/pyanaconda/ui/gui/spokes/user.py
> index 206a8ce..bcf961b 100644
> --- a/pyanaconda/ui/gui/spokes/user.py
> +++ b/pyanaconda/ui/gui/spokes/user.py
> @@ -30,6 +30,7 @@ from pyanaconda.ui.gui import GUIObject
> from pyanaconda.ui.gui.categories.user_settings import UserSettingsCategory
> from pyanaconda.ui.common import FirstbootSpokeMixIn
> from pyanaconda.ui.gui.utils import enlightbox
> +from pyanaconda.iutil import parse_group_str
>
> from pykickstart.constants import FIRSTBOOT_RECONFIG
> from pyanaconda.constants import ANACONDA_ENVIRON, FIRSTBOOT_ENVIRON
> @@ -126,9 +127,12 @@ class AdvancedUserDialog(GUIObject):
> self._user.groups = []
> for group in groups:
> group = group.strip()
> - if group not in self._groupDict:
> - self._groupDict[group] = self.data.GroupData(name =
> group)
> - self._user.groups.append(group)
> + name, gid = parse_group_str(group)
> + if not name or not group:
> + continue
It's kind of pointless to check "not group" here, since "not group" implies
"not name". There would be a point in checking "not group" before calling
parse_group_str().
> + if name not in self._groupDict or self._groupDict[name].gid
> != gid:
> + self._groupDict[name] = self.data.GroupData(name=name,
> gid=gid)
> + self._user.groups.append(name)
>
> #Cancel clicked, window destroyed...
> else:
> diff --git a/tests/pyanaconda_tests/iutil_test.py
> b/tests/pyanaconda_tests/iutil_test.py
> index 06e8433..e1654c5 100644
> --- a/tests/pyanaconda_tests/iutil_test.py
> +++ b/tests/pyanaconda_tests/iutil_test.py
> @@ -372,3 +372,12 @@ class RunProgramTests(unittest.TestCase):
>
> for d, r in dirs:
> self.assertEquals(iutil.parent_dir(d), r)
> +
> + def parse_group_str_test(self):
> + groups = [("acme", "acme", None), ("acme (2911)", "acme", 2911),
> + ("acme(2911)", "acme", 2911),
> + ("acme ()", "acme", None), ("", "", None), ("()", "",
> None),
> + ("(2911)", "", None)]
> +
> + for group_str, name, gid in groups:
> + self.assertEquals(iutil.parse_group_str(group_str), (name, gid))
> --
> 1.9.3
>
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
>
- mulhern
More information about the anaconda-patches
mailing list