[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