Merge lp:~gesha/linaro-license-protection/remove-fallback into lp:~linaro-automation/linaro-license-protection/trunk
- remove-fallback
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 144 |
Proposed branch: | lp:~gesha/linaro-license-protection/remove-fallback |
Merge into: | lp:~linaro-automation/linaro-license-protection/trunk |
Diff against target: |
243 lines (+35/-91) 6 files modified
license_protected_downloads/render_text_files.py (+23/-62) license_protected_downloads/tests/test_render_text_files.py (+11/-21) settings.py (+1/-5) templates/textile_fallbacks/HOWTO_flashfirmware.txt (+0/-1) templates/textile_fallbacks/HOWTO_getsourceandbuild.txt (+0/-1) templates/textile_fallbacks/HOWTO_install.txt (+0/-1) |
To merge this branch: | bzr merge lp:~gesha/linaro-license-protection/remove-fallback |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Данило Шеган (community) | Approve | ||
Review via email: mp+131152@code.launchpad.net |
Commit message
Description of the change
This branch removes fallback support for HOWTOs and adds searching of HOWTOs only in current dir.
Georgy Redkozubov (gesha) wrote : | # |
> 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.
- 146. By Georgy Redkozubov
-
Fixes based on review comments
Данило Шеган (danilo) wrote : | # |
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 MultipleFilesEx
- 147. By Georgy Redkozubov
-
Removed recursive search based on review.
Данило Шеган (danilo) wrote : | # |
Other than the exception still missing the string passed in, looks good. Thanks for keeping up with my requests :)
- 148. By Georgy Redkozubov
-
Added exception explanation.
Preview Diff
1 | === modified file 'license_protected_downloads/render_text_files.py' | |||
2 | --- license_protected_downloads/render_text_files.py 2012-10-23 18:51:49 +0000 | |||
3 | +++ license_protected_downloads/render_text_files.py 2012-10-24 16:10:24 +0000 | |||
4 | @@ -1,5 +1,4 @@ | |||
5 | 1 | import os | 1 | import os |
6 | 2 | import re | ||
7 | 3 | import textile | 2 | import textile |
8 | 4 | import OrderedDict | 3 | import OrderedDict |
9 | 5 | from django.conf import settings | 4 | from django.conf import settings |
10 | @@ -35,15 +34,6 @@ | |||
11 | 35 | title = settings.FILES_MAP[os.path.basename(filepath)] | 34 | title = settings.FILES_MAP[os.path.basename(filepath)] |
12 | 36 | result[title] = cls.render_file(filepath) | 35 | result[title] = cls.render_file(filepath) |
13 | 37 | 36 | ||
14 | 38 | # Switch to fallback data for mandatory files. | ||
15 | 39 | if cls.check_for_manifest_or_tarballs(path): | ||
16 | 40 | for filename in settings.MANDATORY_ANDROID_FILES: | ||
17 | 41 | if settings.FILES_MAP[filename] not in result: | ||
18 | 42 | filepath = os.path.join(settings.TEXTILE_FALLBACK_PATH, | ||
19 | 43 | filename) | ||
20 | 44 | title = settings.FILES_MAP[filename] | ||
21 | 45 | result[title] = cls.render_file(filepath) | ||
22 | 46 | |||
23 | 47 | if not filepaths and len(result) == 0: | 37 | if not filepaths and len(result) == 0: |
24 | 48 | return None | 38 | return None |
25 | 49 | 39 | ||
26 | @@ -67,26 +57,20 @@ | |||
27 | 67 | # Go recursively and find howto's, readme's, hackings, installs. | 57 | # Go recursively and find howto's, readme's, hackings, installs. |
28 | 68 | # If there are more of the same type then one, throw custom error as | 58 | # If there are more of the same type then one, throw custom error as |
29 | 69 | # written above. | 59 | # written above. |
35 | 70 | multiple = 0 | 60 | # Raise MultipleFilesException if files from ANDROID_FILES and |
36 | 71 | howtopath = os.path.join(path, settings.HOWTO_PATH) | 61 | # LINUX_FILES exist in the same dir. |
37 | 72 | androidpaths = cls.dirEntries(howtopath, False, | 62 | |
38 | 73 | settings.ANDROID_FILES) | 63 | androidpaths = cls.dirEntries(path, files_list=settings.ANDROID_FILES) |
39 | 74 | ubuntupaths = cls.dirEntries(path, False, settings.LINUX_FILES) | 64 | ubuntupaths = cls.dirEntries(path, files_list=settings.LINUX_FILES) |
40 | 75 | if len(androidpaths) > 0 and len(ubuntupaths) > 0: | 65 | if len(androidpaths) > 0 and len(ubuntupaths) > 0: |
48 | 76 | raise MultipleFilesException | 66 | # Files from ANDROID_FILES and LINUX_FILES exist in the same dir |
49 | 77 | if len(androidpaths) > 0: | 67 | raise MultipleFilesException("Both Android and Ubuntu HOWTO " \ |
50 | 78 | for filepath in settings.ANDROID_FILES: | 68 | "files are found, which is unsupported.") |
51 | 79 | if len(cls.findall(androidpaths, | 69 | else: |
52 | 80 | lambda x: re.search(filepath, x))) > 1: | 70 | if len(androidpaths) > 0: |
46 | 81 | multiple += 1 | ||
47 | 82 | if multiple == 0: | ||
53 | 83 | return androidpaths | 71 | return androidpaths |
54 | 84 | else: | 72 | else: |
60 | 85 | raise MultipleFilesException | 73 | return ubuntupaths |
56 | 86 | elif len(ubuntupaths) > 0: | ||
57 | 87 | return ubuntupaths | ||
58 | 88 | else: | ||
59 | 89 | return [] | ||
61 | 90 | 74 | ||
62 | 91 | @classmethod | 75 | @classmethod |
63 | 92 | def flatten(cls, l, ltypes=(list, tuple)): | 76 | def flatten(cls, l, ltypes=(list, tuple)): |
64 | @@ -105,29 +89,23 @@ | |||
65 | 105 | return ltype(l) | 89 | return ltype(l) |
66 | 106 | 90 | ||
67 | 107 | @classmethod | 91 | @classmethod |
74 | 108 | def dirEntries(cls, path, subdir, *args): | 92 | def dirEntries(cls, path, files_list=None): |
75 | 109 | ''' Return a list of file names found in directory 'dir_name' | 93 | ''' Return a list of file names found in directory 'path' |
76 | 110 | If 'subdir' is True, recursively access subdirectories under | 94 | 'files_list' are file names to match filenames. Matched file names |
77 | 111 | 'dir_name'. | 95 | are added to the list. |
72 | 112 | Additional arguments, if any, are file names to match filenames. | ||
73 | 113 | Matched file names are added to the list. | ||
78 | 114 | If there are no additional arguments, all files found in the | 96 | If there are no additional arguments, all files found in the |
79 | 115 | directory are added to the list. | 97 | directory are added to the list. |
80 | 116 | ''' | 98 | ''' |
81 | 117 | fileList = [] | 99 | fileList = [] |
91 | 118 | if not os.path.exists(path): | 100 | if os.path.exists(path): |
92 | 119 | return fileList | 101 | for file in os.listdir(path): |
93 | 120 | for file in os.listdir(path): | 102 | dirfile = os.path.join(path, file) |
94 | 121 | dirfile = os.path.join(path, file) | 103 | if os.path.isfile(dirfile): |
95 | 122 | if os.path.isfile(dirfile): | 104 | if not files_list: |
87 | 123 | if not args: | ||
88 | 124 | fileList.append(dirfile) | ||
89 | 125 | else: | ||
90 | 126 | if file in cls.flatten(args): | ||
96 | 127 | fileList.append(dirfile) | 105 | fileList.append(dirfile) |
100 | 128 | # recursively access file names in subdirectories | 106 | else: |
101 | 129 | elif os.path.isdir(dirfile) and subdir: | 107 | if file in cls.flatten(files_list): |
102 | 130 | fileList.extend(cls.dirEntries(dirfile, subdir, *args)) | 108 | fileList.append(dirfile) |
103 | 131 | return fileList | 109 | return fileList |
104 | 132 | 110 | ||
105 | 133 | @classmethod | 111 | @classmethod |
106 | @@ -149,20 +127,3 @@ | |||
107 | 149 | # hence there is an out of range exception | 127 | # hence there is an out of range exception |
108 | 150 | except IndexError: | 128 | except IndexError: |
109 | 151 | return indices | 129 | return indices |
110 | 152 | |||
111 | 153 | @classmethod | ||
112 | 154 | def check_for_manifest_or_tarballs(cls, path): | ||
113 | 155 | ''' Check if there is a MANIFEST file in current path or a tarball. | ||
114 | 156 | Also check if we are currently somewhere in 'android' path. | ||
115 | 157 | This hack is necessary for fallback wiki howto links. | ||
116 | 158 | ''' | ||
117 | 159 | if 'android' in path: | ||
118 | 160 | for filename in os.listdir(path): | ||
119 | 161 | if "MANIFEST" in filename: | ||
120 | 162 | return True | ||
121 | 163 | if "tar.bz2" in filename: | ||
122 | 164 | return True | ||
123 | 165 | if ".img" in filename: | ||
124 | 166 | return True | ||
125 | 167 | |||
126 | 168 | return False | ||
127 | 169 | 130 | ||
128 | === modified file 'license_protected_downloads/tests/test_render_text_files.py' | |||
129 | --- license_protected_downloads/tests/test_render_text_files.py 2012-10-23 12:37:11 +0000 | |||
130 | +++ license_protected_downloads/tests/test_render_text_files.py 2012-10-24 16:10:24 +0000 | |||
131 | @@ -33,29 +33,21 @@ | |||
132 | 33 | self.assertEqual([], | 33 | self.assertEqual([], |
133 | 34 | RenderTextFiles.findall(l, lambda x: re.search(r'1', x))) | 34 | RenderTextFiles.findall(l, lambda x: re.search(r'1', x))) |
134 | 35 | 35 | ||
136 | 36 | def make_temp_dir(self, empty=True, file_list=None, dir=None, subdir=None): | 36 | def make_temp_dir(self, empty=True, file_list=None, dir=None): |
137 | 37 | path = tempfile.mkdtemp(dir=dir) | 37 | path = tempfile.mkdtemp(dir=dir) |
138 | 38 | if not empty: | 38 | if not empty: |
139 | 39 | if file_list: | 39 | if file_list: |
140 | 40 | if subdir: | ||
141 | 41 | movepath = os.path.join(path, settings.HOWTO_PATH) | ||
142 | 42 | os.makedirs(movepath) | ||
143 | 43 | else: | ||
144 | 44 | movepath = path | ||
145 | 45 | for file in file_list: | 40 | for file in file_list: |
148 | 46 | handle, fname = tempfile.mkstemp(dir=movepath) | 41 | handle, fname = tempfile.mkstemp(dir=path) |
149 | 47 | shutil.move(fname, os.path.join(movepath, file)) | 42 | shutil.move(fname, os.path.join(path, file)) |
150 | 48 | return path | 43 | return path |
151 | 49 | 44 | ||
152 | 50 | def test_find_relevant_files_android(self): | 45 | def test_find_relevant_files_android(self): |
153 | 51 | path = self.make_temp_dir(empty=False, | 46 | path = self.make_temp_dir(empty=False, |
156 | 52 | file_list=settings.ANDROID_FILES, | 47 | file_list=settings.ANDROID_FILES) |
155 | 53 | subdir=settings.HOWTO_PATH) | ||
157 | 54 | full_android_files = [] | 48 | full_android_files = [] |
158 | 55 | for file in settings.ANDROID_FILES: | 49 | for file in settings.ANDROID_FILES: |
162 | 56 | full_android_files.append(os.path.join(path, | 50 | full_android_files.append(os.path.join(path, file)) |
160 | 57 | settings.HOWTO_PATH, | ||
161 | 58 | file)) | ||
163 | 59 | self.assertEqual(sorted(full_android_files), | 51 | self.assertEqual(sorted(full_android_files), |
164 | 60 | sorted(RenderTextFiles.find_relevant_files(path))) | 52 | sorted(RenderTextFiles.find_relevant_files(path))) |
165 | 61 | 53 | ||
166 | @@ -63,24 +55,22 @@ | |||
167 | 63 | path = self.make_temp_dir() | 55 | path = self.make_temp_dir() |
168 | 64 | full_path = self.make_temp_dir(empty=False, | 56 | full_path = self.make_temp_dir(empty=False, |
169 | 65 | file_list=settings.ANDROID_FILES, | 57 | file_list=settings.ANDROID_FILES, |
171 | 66 | dir=path, subdir=settings.HOWTO_PATH) | 58 | dir=path) |
172 | 67 | full_android_files = [] | 59 | full_android_files = [] |
173 | 68 | for file in settings.ANDROID_FILES: | 60 | for file in settings.ANDROID_FILES: |
177 | 69 | full_android_files.append(os.path.join(full_path, | 61 | full_android_files.append(os.path.join(full_path, file)) |
175 | 70 | settings.HOWTO_PATH, | ||
176 | 71 | file)) | ||
178 | 72 | self.assertEqual([], | 62 | self.assertEqual([], |
179 | 73 | sorted(RenderTextFiles.find_relevant_files(path))) | 63 | sorted(RenderTextFiles.find_relevant_files(path))) |
180 | 74 | self.assertEqual(sorted(full_android_files), | 64 | self.assertEqual(sorted(full_android_files), |
181 | 75 | sorted(RenderTextFiles.find_relevant_files(full_path))) | 65 | sorted(RenderTextFiles.find_relevant_files(full_path))) |
182 | 76 | 66 | ||
183 | 77 | def test_find_relevant_files_android_and_ubuntu_samedir(self): | 67 | def test_find_relevant_files_android_and_ubuntu_samedir(self): |
186 | 78 | path = self.make_temp_dir(empty=False, file_list=settings.LINUX_FILES) | 68 | path = self.make_temp_dir(empty=False, |
187 | 79 | os.makedirs(os.path.join(path, settings.HOWTO_PATH)) | 69 | file_list=settings.LINUX_FILES + settings.ANDROID_FILES) |
188 | 80 | full_files = [] | 70 | full_files = [] |
189 | 81 | for file in settings.ANDROID_FILES: | 71 | for file in settings.ANDROID_FILES: |
192 | 82 | full_files.append(os.path.join(path, settings.HOWTO_PATH, file)) | 72 | full_files.append(os.path.join(path, file)) |
193 | 83 | open(os.path.join(path, settings.HOWTO_PATH, file), 'w').close() | 73 | open(os.path.join(path, file), 'w').close() |
194 | 84 | for file in settings.LINUX_FILES: | 74 | for file in settings.LINUX_FILES: |
195 | 85 | full_files.append(os.path.join(path, file)) | 75 | full_files.append(os.path.join(path, file)) |
196 | 86 | with self.assertRaises(MultipleFilesException): | 76 | with self.assertRaises(MultipleFilesException): |
197 | 87 | 77 | ||
198 | === modified file 'settings.py' | |||
199 | --- settings.py 2012-10-23 13:07:01 +0000 | |||
200 | +++ settings.py 2012-10-24 16:10:24 +0000 | |||
201 | @@ -163,22 +163,18 @@ | |||
202 | 163 | ) | 163 | ) |
203 | 164 | 164 | ||
204 | 165 | # Render TEXTILE files settings. | 165 | # Render TEXTILE files settings. |
205 | 166 | HOWTO_PATH = 'howto/' | ||
206 | 167 | |||
207 | 168 | LINUX_FILES = ('README', | 166 | LINUX_FILES = ('README', |
208 | 169 | 'INSTALL', | 167 | 'INSTALL', |
209 | 170 | 'HACKING', | 168 | 'HACKING', |
210 | 171 | 'FIRMWARE', | 169 | 'FIRMWARE', |
211 | 172 | 'RTSM') | 170 | 'RTSM') |
212 | 171 | |||
213 | 173 | ANDROID_FILES = ('HOWTO_releasenotes.txt', | 172 | ANDROID_FILES = ('HOWTO_releasenotes.txt', |
214 | 174 | 'HOWTO_install.txt', | 173 | 'HOWTO_install.txt', |
215 | 175 | 'HOWTO_getsourceandbuild.txt', | 174 | 'HOWTO_getsourceandbuild.txt', |
216 | 176 | 'HOWTO_flashfirmware.txt', | 175 | 'HOWTO_flashfirmware.txt', |
217 | 177 | 'HOWTO_rtsm.txt') | 176 | 'HOWTO_rtsm.txt') |
218 | 178 | 177 | ||
219 | 179 | MANDATORY_ANDROID_FILES = ('HOWTO_install.txt', | ||
220 | 180 | 'HOWTO_getsourceandbuild.txt') | ||
221 | 181 | |||
222 | 182 | FILES_MAP = {'HOWTO_releasenotes.txt': 'Release Notes', | 178 | FILES_MAP = {'HOWTO_releasenotes.txt': 'Release Notes', |
223 | 183 | 'HOWTO_install.txt': 'Binary Image Installation', | 179 | 'HOWTO_install.txt': 'Binary Image Installation', |
224 | 184 | 'HOWTO_getsourceandbuild.txt': 'Building From Source', | 180 | 'HOWTO_getsourceandbuild.txt': 'Building From Source', |
225 | 185 | 181 | ||
226 | === removed directory 'templates/textile_fallbacks' | |||
227 | === removed file 'templates/textile_fallbacks/HOWTO_flashfirmware.txt' | |||
228 | --- templates/textile_fallbacks/HOWTO_flashfirmware.txt 2012-10-19 11:06:33 +0000 | |||
229 | +++ templates/textile_fallbacks/HOWTO_flashfirmware.txt 1970-01-01 00:00:00 +0000 | |||
230 | @@ -1,1 +0,0 @@ | |||
231 | 1 | 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 | ||
232 | 2 | 0 | ||
233 | === removed file 'templates/textile_fallbacks/HOWTO_getsourceandbuild.txt' | |||
234 | --- templates/textile_fallbacks/HOWTO_getsourceandbuild.txt 2012-10-19 11:06:33 +0000 | |||
235 | +++ templates/textile_fallbacks/HOWTO_getsourceandbuild.txt 1970-01-01 00:00:00 +0000 | |||
236 | @@ -1,1 +0,0 @@ | |||
237 | 1 | 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 | ||
238 | 2 | 0 | ||
239 | === removed file 'templates/textile_fallbacks/HOWTO_install.txt' | |||
240 | --- templates/textile_fallbacks/HOWTO_install.txt 2012-10-19 11:06:33 +0000 | |||
241 | +++ templates/textile_fallbacks/HOWTO_install.txt 1970-01-01 00:00:00 +0000 | |||
242 | @@ -1,1 +0,0 @@ | |||
243 | 1 | 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 |
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?