Merge lp:~gesha/linaro-license-protection/remove-fallback into lp:~linaro-automation/linaro-license-protection/trunk

[Georgy Redkozubov]

This branch removes fallback support for HOWTOs and adds searching of HOWTOs only in current dir.

[Данило Шеган (danilo)]

In the call to cls.dirEntries, it is unclear what do arguments represent (especially "False"). Python does have one of my favourite features for readability, which is passing arguments by keywords: please use that.

Where you check

         if len(androidpaths) > 0 and len(ubuntupaths) > 0:

you could add an "else" block and get rid of the "else" block in the if below (and replace "elif" simply with "else"): not a big deal, but will move "corner" cases to one if, and actual handling to another.

In tests, I wonder if subdir argument is still needed at all for make_temp_dir?

review: Needs Fixing

[Georgy Redkozubov (gesha)]

> In the call to cls.dirEntries, it is unclear what do arguments represent
> (especially "False"). Python does have one of my favourite features for
> readability, which is passing arguments by keywords: please use that.

Not sure is you meant what I did, but anyway fixed.

>
> Where you check
>
> if len(androidpaths) > 0 and len(ubuntupaths) > 0:
>
> you could add an "else" block and get rid of the "else" block in the if below
> (and replace "elif" simply with "else"): not a big deal, but will move
> "corner" cases to one if, and actual handling to another.

Done

>
> In tests, I wonder if subdir argument is still needed at all for
> make_temp_dir?

Thanks for pointing to this, I forget to remove initially.

[lp:~gesha/linaro-license-protection/remove-fallback]

146. By Georgy Redkozubov on 2012-10-24

Fixes based on review comments

[Данило Шеган (danilo)]

Ok, now that it's clear that second parameter to dirEntries is "subdir", I suppose we can get rid of that as well :)

Also, please provide a string explanation of the error condition where we raise MultipleFilesException.

[lp:~gesha/linaro-license-protection/remove-fallback]

147. By Georgy Redkozubov on 2012-10-24

Removed recursive search based on review.

[Данило Шеган (danilo)]

Other than the exception still missing the string passed in, looks good. Thanks for keeping up with my requests :)

review: Approve

[lp:~gesha/linaro-license-protection/remove-fallback]

148. By Georgy Redkozubov on 2012-10-24

Added exception explanation.

=== modified file 'license_protected_downloads/render_text_files.py'
--- license_protected_downloads/render_text_files.py	2012-10-23 18:51:49 +0000
+++ license_protected_downloads/render_text_files.py	2012-10-24 16:10:24 +0000
@@ -1,5 +1,4 @@
 import os
-import re
 import textile
 import OrderedDict
 from django.conf import settings
@@ -35,15 +34,6 @@
                 title = settings.FILES_MAP[os.path.basename(filepath)]
                 result[title] = cls.render_file(filepath)
 
-        # Switch to fallback data for mandatory files.
-        if cls.check_for_manifest_or_tarballs(path):
-            for filename in settings.MANDATORY_ANDROID_FILES:
-                if settings.FILES_MAP[filename] not in result:
-                    filepath = os.path.join(settings.TEXTILE_FALLBACK_PATH,
-                                            filename)
-                    title = settings.FILES_MAP[filename]
-                    result[title] = cls.render_file(filepath)
-
         if not filepaths and len(result) == 0:
             return None
 
@@ -67,26 +57,20 @@
         # Go recursively and find howto's, readme's, hackings, installs.
         # If there are more of the same type then one, throw custom error as
         # written above.
-        multiple = 0
-        howtopath = os.path.join(path, settings.HOWTO_PATH)
-        androidpaths = cls.dirEntries(howtopath, False,
-                                      settings.ANDROID_FILES)
-        ubuntupaths = cls.dirEntries(path, False, settings.LINUX_FILES)
+        # Raise MultipleFilesException if files from ANDROID_FILES and
+        # LINUX_FILES exist in the same dir.
+
+        androidpaths = cls.dirEntries(path, files_list=settings.ANDROID_FILES)
+        ubuntupaths = cls.dirEntries(path, files_list=settings.LINUX_FILES)
         if len(androidpaths) > 0 and len(ubuntupaths) > 0:
-            raise MultipleFilesException
-        if len(androidpaths) > 0:
-            for filepath in settings.ANDROID_FILES:
-                if len(cls.findall(androidpaths,
-                                   lambda x: re.search(filepath, x))) > 1:
-                    multiple += 1
-            if multiple == 0:
+            # Files from ANDROID_FILES and LINUX_FILES exist in the same dir
+            raise MultipleFilesException("Both Android and Ubuntu HOWTO " \
+             "files are found, which is unsupported.")
+        else:
+            if len(androidpaths) > 0:
                 return androidpaths
             else:
-                raise MultipleFilesException
-        elif len(ubuntupaths) > 0:
-            return ubuntupaths
-        else:
-            return []
+                return ubuntupaths
 
     @classmethod
     def flatten(cls, l, ltypes=(list, tuple)):
@@ -105,29 +89,23 @@
         return ltype(l)
 
     @classmethod
-    def dirEntries(cls, path, subdir, *args):
-        ''' Return a list of file names found in directory 'dir_name'
-            If 'subdir' is True, recursively access subdirectories under
-            'dir_name'.
-            Additional arguments, if any, are file names to match filenames.
-            Matched file names are added to the list.
+    def dirEntries(cls, path, files_list=None):
+        ''' Return a list of file names found in directory 'path'
+            'files_list' are file names to match filenames. Matched file names
+            are added to the list.
             If there are no additional arguments, all files found in the
             directory are added to the list.
         '''
         fileList = []
-        if not os.path.exists(path):
-            return fileList
-        for file in os.listdir(path):
-            dirfile = os.path.join(path, file)
-            if os.path.isfile(dirfile):
-                if not args:
-                    fileList.append(dirfile)
-                else:
-                    if file in cls.flatten(args):
+        if os.path.exists(path):
+            for file in os.listdir(path):
+                dirfile = os.path.join(path, file)
+                if os.path.isfile(dirfile):
+                    if not files_list:
                         fileList.append(dirfile)
-            # recursively access file names in subdirectories
-            elif os.path.isdir(dirfile) and subdir:
-                fileList.extend(cls.dirEntries(dirfile, subdir, *args))
+                    else:
+                        if file in cls.flatten(files_list):
+                            fileList.append(dirfile)
         return fileList
 
     @classmethod
@@ -149,20 +127,3 @@
             # hence there is an out of range exception
             except IndexError:
                 return indices
-
-    @classmethod
-    def check_for_manifest_or_tarballs(cls, path):
-        ''' Check if there is a MANIFEST file in current path or a tarball.
-        Also check if we are currently somewhere in 'android' path.
-        This hack is necessary for fallback wiki howto links.
-        '''
-        if 'android' in path:
-            for filename in os.listdir(path):
-                if "MANIFEST" in filename:
-                    return True
-                if "tar.bz2" in filename:
-                    return True
-                if ".img" in filename:
-                    return True
-
-        return False

=== modified file 'license_protected_downloads/tests/test_render_text_files.py'
--- license_protected_downloads/tests/test_render_text_files.py	2012-10-23 12:37:11 +0000
+++ license_protected_downloads/tests/test_render_text_files.py	2012-10-24 16:10:24 +0000
@@ -33,29 +33,21 @@
         self.assertEqual([],
             RenderTextFiles.findall(l, lambda x: re.search(r'1', x)))
 
-    def make_temp_dir(self, empty=True, file_list=None, dir=None, subdir=None):
+    def make_temp_dir(self, empty=True, file_list=None, dir=None):
         path = tempfile.mkdtemp(dir=dir)
         if not empty:
             if file_list:
-                if subdir:
-                    movepath = os.path.join(path, settings.HOWTO_PATH)
-                    os.makedirs(movepath)
-                else:
-                    movepath = path
                 for file in file_list:
-                    handle, fname = tempfile.mkstemp(dir=movepath)
-                    shutil.move(fname, os.path.join(movepath, file))
+                    handle, fname = tempfile.mkstemp(dir=path)
+                    shutil.move(fname, os.path.join(path, file))
         return path
 
     def test_find_relevant_files_android(self):
         path = self.make_temp_dir(empty=False,
-                                  file_list=settings.ANDROID_FILES,
-                                  subdir=settings.HOWTO_PATH)
+                                  file_list=settings.ANDROID_FILES)
         full_android_files = []
         for file in settings.ANDROID_FILES:
-            full_android_files.append(os.path.join(path,
-                                                   settings.HOWTO_PATH,
-                                                   file))
+            full_android_files.append(os.path.join(path, file))
         self.assertEqual(sorted(full_android_files),
             sorted(RenderTextFiles.find_relevant_files(path)))
 
@@ -63,24 +55,22 @@
         path = self.make_temp_dir()
         full_path = self.make_temp_dir(empty=False,
                                        file_list=settings.ANDROID_FILES,
-                                       dir=path, subdir=settings.HOWTO_PATH)
+                                       dir=path)
         full_android_files = []
         for file in settings.ANDROID_FILES:
-            full_android_files.append(os.path.join(full_path,
-                                                   settings.HOWTO_PATH,
-                                                   file))
+            full_android_files.append(os.path.join(full_path, file))
         self.assertEqual([],
             sorted(RenderTextFiles.find_relevant_files(path)))
         self.assertEqual(sorted(full_android_files),
             sorted(RenderTextFiles.find_relevant_files(full_path)))
 
     def test_find_relevant_files_android_and_ubuntu_samedir(self):
-        path = self.make_temp_dir(empty=False, file_list=settings.LINUX_FILES)
-        os.makedirs(os.path.join(path, settings.HOWTO_PATH))
+        path = self.make_temp_dir(empty=False,
+                    file_list=settings.LINUX_FILES + settings.ANDROID_FILES)
         full_files = []
         for file in settings.ANDROID_FILES:
-            full_files.append(os.path.join(path, settings.HOWTO_PATH, file))
-            open(os.path.join(path, settings.HOWTO_PATH, file), 'w').close()
+            full_files.append(os.path.join(path, file))
+            open(os.path.join(path, file), 'w').close()
         for file in settings.LINUX_FILES:
             full_files.append(os.path.join(path, file))
         with self.assertRaises(MultipleFilesException):

=== modified file 'settings.py'
--- settings.py	2012-10-23 13:07:01 +0000
+++ settings.py	2012-10-24 16:10:24 +0000
@@ -163,22 +163,18 @@
 )
 
 # Render TEXTILE files settings.
-HOWTO_PATH = 'howto/'
-
 LINUX_FILES = ('README',
                'INSTALL',
                'HACKING',
                'FIRMWARE',
                'RTSM')
+
 ANDROID_FILES = ('HOWTO_releasenotes.txt',
                  'HOWTO_install.txt',
                  'HOWTO_getsourceandbuild.txt',
                  'HOWTO_flashfirmware.txt',
                  'HOWTO_rtsm.txt')
 
-MANDATORY_ANDROID_FILES = ('HOWTO_install.txt',
-                           'HOWTO_getsourceandbuild.txt')
-
 FILES_MAP = {'HOWTO_releasenotes.txt': 'Release Notes',
              'HOWTO_install.txt': 'Binary Image Installation',
              'HOWTO_getsourceandbuild.txt': 'Building From Source',

=== removed directory 'templates/textile_fallbacks'
=== removed file 'templates/textile_fallbacks/HOWTO_flashfirmware.txt'
--- templates/textile_fallbacks/HOWTO_flashfirmware.txt	2012-10-19 11:06:33 +0000
+++ templates/textile_fallbacks/HOWTO_flashfirmware.txt	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-Please read "Flash Firmware Wiki page":https://wiki.linaro.org/Platform/Android/FlashFirmware <br><br>For more info, check out this "page":https://wiki.linaro.org/Platform/Android

=== removed file 'templates/textile_fallbacks/HOWTO_getsourceandbuild.txt'
--- templates/textile_fallbacks/HOWTO_getsourceandbuild.txt	2012-10-19 11:06:33 +0000
+++ templates/textile_fallbacks/HOWTO_getsourceandbuild.txt	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-Please read these two pages: "Get Source page":https://wiki.linaro.org/Platform/Android/GetSource and "Build Source page":https://wiki.linaro.org/Platform/Android/BuildSource <br><br>For more info, check out this "page":https://wiki.linaro.org/Platform/Android

=== removed file 'templates/textile_fallbacks/HOWTO_install.txt'
--- templates/textile_fallbacks/HOWTO_install.txt	2012-10-19 11:06:33 +0000
+++ templates/textile_fallbacks/HOWTO_install.txt	1970-01-01 00:00:00 +0000
@@ -1,1 +0,0 @@
-Please read "Image Installation Wiki page":https://wiki.linaro.org/Platform/Android/ImageInstallation <br><br>For more info, check out this "page":https://wiki.linaro.org/Platform/Android