[rhel7/master/f21 1/2 V2] Create missing parent directories for user's home directory (#1163775)

Anne Mulhern amulhern at redhat.com
Mon Nov 17 12:44:14 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:42 PM
> Subject: [rhel7/master/f21 1/2 V2] Create missing parent directories for	user's home directory (#1163775)
> 
> libuser doesn't create parent directories. This simplifies the homedir
> creation slightly and adds a utility function, with a test, that returns
> the home directories parents, suitable for passing to mkdirChain.

"the home directories parents" would be better as "the directory's parent's path"

or something like that.

> 
> Resolves: rhbz#1163775
> ---
>  pyanaconda/iutil.py                  |  4 ++++
>  pyanaconda/ui/gui/spokes/user.py     |  2 ++
>  pyanaconda/users.py                  | 13 ++++++++-----
>  tests/pyanaconda_tests/iutil_test.py |  9 +++++++++
>  4 files changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> index ad09850..7e3dde3 100644
> --- a/pyanaconda/iutil.py
> +++ b/pyanaconda/iutil.py
> @@ -883,3 +883,7 @@ def save_screenshots():
>  
>      except OSError:
>          log.exception("saving screenshots to installed system failed")
> +
> +def parent_dir(directory):
> +    """Return the parent directories"""

"parent directories" -> "parent's path"

> +    return "/".join(os.path.normpath(directory).split("/")[:-1])
> diff --git a/pyanaconda/ui/gui/spokes/user.py
> b/pyanaconda/ui/gui/spokes/user.py
> index e40b413..206a8ce 100644
> --- a/pyanaconda/ui/gui/spokes/user.py
> +++ b/pyanaconda/ui/gui/spokes/user.py
> @@ -107,6 +107,8 @@ class AdvancedUserDialog(GUIObject):
>          if rc == 1:
>              if self.builder.get_object("c_home").get_active():
>                  self._user.homedir =
>                  self.builder.get_object("t_home").get_text()
> +                if not self._user.homedir.startswith("/"):

Would be nicer to use os.path.isabs() here.

> +                    self._user.homedir = "/" + self._user.homedir

Would be nicer to use os.path.join() here.

>              else:
>                  self._user.homedir = None
>  
> diff --git a/pyanaconda/users.py b/pyanaconda/users.py
> index 0eb7435..18021ea 100644
> --- a/pyanaconda/users.py
> +++ b/pyanaconda/users.py
> @@ -287,11 +287,14 @@ class Users:
>                  userEnt.set(libuser.GIDNUMBER,
>                  [groupEnt.get(libuser.GIDNUMBER)[0]] +
>                              map(lambda grp: grp.get(libuser.GIDNUMBER)[0],
>                              grpLst))
>  
> -                if kwargs.get("homedir", False):
> -                    userEnt.set(libuser.HOMEDIRECTORY, kwargs["homedir"])
> -                else:
> -                    iutil.mkdirChain('/home')
> -                    userEnt.set(libuser.HOMEDIRECTORY, "/home/" + user_name)
> +                homedir = kwargs.get("homedir", None)
> +                if not homedir:
> +                    homedir = "/home/" + user_name

Could make this os.path.join() while you're changing it anyway.

> +                # libuser expects the parent directory tree to exist.
> +                parent_dir = iutil.parent_dir(homedir)
> +                if parent_dir:
> +                    iutil.mkdirChain(parent_dir)
> +                userEnt.set(libuser.HOMEDIRECTORY, homedir)
>  
>                  if kwargs.get("shell", False):
>                      userEnt.set(libuser.LOGINSHELL, kwargs["shell"])
> diff --git a/tests/pyanaconda_tests/iutil_test.py
> b/tests/pyanaconda_tests/iutil_test.py
> index bd7aced..06e8433 100644
> --- a/tests/pyanaconda_tests/iutil_test.py
> +++ b/tests/pyanaconda_tests/iutil_test.py
> @@ -363,3 +363,12 @@ class RunProgramTests(unittest.TestCase):
>          self.assertFalse(iutil.cmp_obj_attrs(a, b, ["b", "c"]))
>          self.assertFalse(iutil.cmp_obj_attrs(b, a, ["b", "c"]))
>          self.assertFalse(iutil.cmp_obj_attrs(b, a, ["c", "b"]))
> +
> +    def parent_dir_test(self):
> +        """Test the parent_dir function"""
> +        dirs = [("", ""), ("/", ""), ("/home/", ""), ("/home/bcl", "/home"),
> ("home/bcl", "home"),
> +                ("/home/bcl/", "/home"), ("/home/extra/bcl", "/home/extra"),
> +                ("/home/extra/bcl/", "/home/extra"), ("/home/extra/../bcl/",
> "/home")]
> +
> +        for d, r in dirs:
> +            self.assertEquals(iutil.parent_dir(d), r)
> --
> 1.9.3
> 
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> 

Otherwise, ack.

- mulhern


More information about the anaconda-patches mailing list