Re: [PATCH] Perform recursive copying of driver disk RPM repo contents

Radek Vykydal <[email protected]> Tue, 28 Feb 2017 11:51:26 +0100
Newsgroups gmane.linux.redhat.anaconda.devel
Message-ID <[email protected]>
Thank you for your patch.

Would you mind creating a pull request for rhel7-branch
of https://github.com/rhinstaller/anaconda ? This would help to have the 
patch reviewed in scope of our common work flow.

You can add the patch for the "file file" typo in the same PR as a 
separate "Related: rhbz#1425760" patch.

Radek

On 24.2.2017 21:36, Eugene Syromiatnikov wrote:
> Commit anaconda-25.20-1-100-ge0e86f4 introduced a new way of copying RPM
> repository data from a driver disk, which apparently is not recursive.
> Since repository metadata is stored in a separate directory "repodata",
> this now produces a warning like this:
>
>      DD: WARNING: DD repo content not a file: /media/DD-1/rpms/x86_64/repodata
>
> and skips repodata directory entirely. Luckily, it is generated later by
> the code in pyanaconda.packaging,PackagePayload.addDriverRepos().
>
> Unfortunately, test case was also written in assumption that repodata is
> just a file and not a directory, so it hasn't caught it.
>
> Resolves: #rhbz1425760
>
> * dracut/driver_updates.py (save_repo): Implement recursive copying.
> * tests/dracut_tests/test_driver_updates.py (makerepodata): New function
> for populating fake repo metadata.
> (TestSaveRepo.test_folder_repo): Update test in order to check for the
> case of repodata being a directory.
> ---
>   dracut/driver_updates.py                  | 17 ++++++++++-------
>   tests/dracut_tests/test_driver_updates.py | 16 ++++++++++++++--
>   2 files changed, 24 insertions(+), 9 deletions(-)
>
> diff --git a/dracut/driver_updates.py b/dracut/driver_updates.py
> index 3c0b714..488fe29 100755
> --- a/dracut/driver_updates.py
> +++ b/dracut/driver_updates.py
> @@ -272,13 +272,16 @@ def save_repo(repo, target="/run/install"):
>       if os.path.isfile(repo):
>           shutil.copy2(repo, newdir)
>       elif os.path.isdir(repo):
> -        for item in os.listdir(repo):
> -            item_path = os.path.join(repo, item)
> -            if os.path.isfile(item_path):
> -                log.debug("copying %s to %s", item_path, newdir)
> -                shutil.copy2(item_path, newdir)
> -            else:
> -                log.warning("DD repo content not a file: %s", item_path)
> +        for root, dirs, files in os.walk(repo):
> +            dest_path = os.path.join(newdir, os.path.relpath(root, repo))
> +            for file in files:
> +                item_path = os.path.join(repo, root, file)
> +                log.debug("copying %s to %s", item_path, dest_path)
> +                shutil.copy2(item_path, dest_path)
> +            for dir in dirs:
> +                item_path = os.path.join(dest_path, dir)
> +                log.debug("creating %s", item_path)
> +                os.mkdir(item_path)
>       else:
>           log.error("ERROR: DD repository needs to be a file or a directory: %s",
>                     repo)
> diff --git a/tests/dracut_tests/test_driver_updates.py b/tests/dracut_tests/test_driver_updates.py
> index 55120c9..3f1fb30 100644
> --- a/tests/dracut_tests/test_driver_updates.py
> +++ b/tests/dracut_tests/test_driver_updates.py
> @@ -250,6 +250,14 @@ def makerepo(topdir, desc=None):
>       makedir(topdir+'/rpms/'+ARCH)
>   
>   
> +def makerepodata(topdir):
> +    makedir(topdir + '/repodata/')
> +    makefile(topdir + '/repodata/repomd.xml')
> +    makefile(topdir + '/repodata/filelists.xml.gz')
> +    makefile(topdir + '/repodata/primary.xml.gz')
> +    makefile(topdir + '/repodata/other.xml.gz')
> +
> +
>   class TestFindRepos(FileTestCaseBase):
>       def test_basic(self):
>           """find_repos: return RPM dir if a valid repo is found"""
> @@ -272,12 +280,16 @@ class TestSaveRepo(FileTestCaseBase):
>           """save_repo: copies directory contents to /run/install/DD-X"""
>           makerepo(self.srcdir)
>           repo = find_repos(self.srcdir)[0]
> -        makefile(repo + '/repodata')
> +        makerepodata(repo)
>           makefile(repo + '/fake-something1.rpm')
>           makefile(repo + '/fake-something2.rpm')
>           makefile(repo + '/fake-something3.rpm')
>           saved = save_repo(repo, target=self.destdir)
> -        expected_files = set(["fake-something1.rpm", "fake-something2.rpm", "fake-something3.rpm", "repodata"])
> +        expected_files = set(["fake-something1.rpm", "fake-something2.rpm",
> +                              "fake-something3.rpm", "repodata/repomd.xml",
> +                              "repodata/filelists.xml.gz",
> +                              "repodata/primary.xml.gz",
> +                              "repodata/other.xml.gz"])
>           self.assertEqual(set(listfiles(saved)), expected_files)
>           self.assertEqual(saved, os.path.join(self.destdir, "DD-1"))
>