1 |
Dne 30.8.2011 09:35, Michał Górny napsal(a): |
2 |
> On Tue, 30 Aug 2011 09:11:40 +0200 |
3 |
> Tomáš Chvátal<scarabeus@g.o> wrote: |
4 |
> |
5 |
>> @@ -66,80 +52,234 @@ |
6 |
>> |
7 |
>> # @ECLASS-VARIABLE: CHECKREQS_MEMORY |
8 |
>> # @DESCRIPTION: |
9 |
>> -# How much RAM is needed in MB? |
10 |
>> +# @DEAULT_UNSET |
11 |
>> +# How much RAM is needed? |
12 |
> |
13 |
> Typo. Also, shouldn't defaults go before @DESCRIPTION: ? |
14 |
Yeah sedding it failed :) |
15 |
> |
16 |
>> # @ECLASS-VARIABLE: CHECKREQS_DISK_BUILD |
17 |
>> # @DESCRIPTION: |
18 |
>> -# How much diskspace is needed to build the package? In MB |
19 |
>> +# @DEAULT_UNSET |
20 |
>> +# How much diskspace is needed to build the package? |
21 |
> |
22 |
> Ditto. |
23 |
> |
24 |
>> # @ECLASS-VARIABLE: CHECKREQS_DISK_USR |
25 |
>> # @DESCRIPTION: |
26 |
>> -# How much space in /usr is needed to install the package? In MB |
27 |
>> +# @DEAULT_UNSET |
28 |
>> +# How much space in /usr is needed to install the package? |
29 |
> |
30 |
> Ditto. |
31 |
> |
32 |
>> # @ECLASS-VARIABLE: CHECKREQS_DISK_VAR |
33 |
>> # @DESCRIPTION: |
34 |
>> -# How much space is needed in /var? In MB |
35 |
>> +# @DEAULT_UNSET |
36 |
>> +# How much space is needed in /var? |
37 |
> |
38 |
> Ditto. |
39 |
> |
40 |
>> +CHECKREQS_EXPORTED_FUNCTIONS="pkg_setup" |
41 |
>> +case "${EAPI:-0}" in |
42 |
>> + 0|1|2|3) ;; |
43 |
>> + 4) CHECKREQS_EXPORTED_FUNCTIONS="${EXPORTED_FUNCTIONS} |
44 |
>> pkg_pretend" ;; |
45 |
> |
46 |
> Not the same var. |
47 |
> |
48 |
>> + *) die "EAPI=${EAPI} is not supported" ;; |
49 |
>> +esac |
50 |
> |
51 |
> CHECKREQS_EXPORTED_FUNCTIONS is not used anywhere. |
52 |
Fixed. |
53 |
> |
54 |
>> # @FUNCTION: check_reqs |
55 |
>> # @DESCRIPTION: |
56 |
>> -# Checks the requirements given in the specific variables. If not |
57 |
>> reached, -# either prints a warning or dies. |
58 |
>> +# Obsolete function executing all the checks and priting out results |
59 |
>> check_reqs() { |
60 |
>> - [[ -n "${1}" ]]&& die "Usage: check_reqs" |
61 |
>> + debug-print-function ${FUNCNAME} "$@" |
62 |
>> + |
63 |
>> + echo |
64 |
>> + ewarn "QA: Package calling old ${FUNCNAME} function." |
65 |
>> + ewarn "QA: Please file a bug against the package." |
66 |
>> + ewarn "QA: It should call check-reqs_pkg_pretend and |
67 |
>> check-reqs_pkg_setup" |
68 |
>> + ewarn "QA: and possibly use EAPI=4 or later." |
69 |
>> + echo |
70 |
>> + |
71 |
>> + check-reqs_pkg_setup "$@" |
72 |
>> +} |
73 |
>> |
74 |
>> - export CHECKREQS_NEED_SLEEP="" CHECKREQS_NEED_DIE="" |
75 |
>> - if [[ "$CHECKREQS_ACTION" != "ignore" ]] ; then |
76 |
>> - [[ -n "$CHECKREQS_MEMORY" ]]&& check_build_memory |
77 |
>> - [[ -n "$CHECKREQS_DISK_BUILD" ]]&& check_build_disk |
78 |
>> \ |
79 |
>> - "${T}" "${CHECKREQS_DISK_BUILD}" |
80 |
>> - [[ -n "$CHECKREQS_DISK_USR" ]]&& check_build_disk \ |
81 |
>> - "${ROOT}/usr" "${CHECKREQS_DISK_USR}" |
82 |
>> - [[ -n "$CHECKREQS_DISK_VAR" ]]&& check_build_disk \ |
83 |
>> - "${ROOT}/var" "${CHECKREQS_DISK_VAR}" |
84 |
>> +# @FUNCTION: check-reqs_pkg_setup |
85 |
>> +# @DESCRIPTION: |
86 |
>> +# Exported function running the resources checks in pkg_setup phase. |
87 |
>> +# It should be run in both phases to ensure condition changes between |
88 |
>> +# pkg_pretend and pkg_setup won't affect the build. |
89 |
>> +check-reqs_pkg_setup() { |
90 |
>> + debug-print-function ${FUNCNAME} "$@" |
91 |
>> + |
92 |
>> + check-reqs_prepare |
93 |
>> + check-reqs_run |
94 |
>> + check-reqs_output |
95 |
>> +} |
96 |
>> + |
97 |
>> +# @FUNCTION: check-reqs_pkg_pretend |
98 |
>> +# @DESCRIPTION: |
99 |
>> +# Exported function running the resources checks in pkg_pretend |
100 |
>> phase. +check-reqs_pkg_pretend() { |
101 |
>> + debug-print-function ${FUNCNAME} "$@" |
102 |
>> + |
103 |
>> + check-reqs_pkg_setup "$@" |
104 |
>> +} |
105 |
>> + |
106 |
>> +# @FUNCTION: check-reqs_prepare |
107 |
>> +# @DESCRIPTION: |
108 |
>> +# Internal function that checks the variables that should be defined. |
109 |
>> +check-reqs_prepare() { |
110 |
>> + debug-print-function ${FUNCNAME} "$@" |
111 |
>> + |
112 |
>> + if [[ -z ${CHECKREQS_MEMORY}&& |
113 |
>> + -z ${CHECKREQS_DISK_BUILD}&& |
114 |
>> + -z ${CHECKREQS_DISK_USR}&& |
115 |
>> + -z ${CHECKREQS_DISK_VAR} ]]; then |
116 |
>> + eerror "Set some check-reqs eclass variables if you |
117 |
>> want to use it." |
118 |
>> + eerror "If you are user and see this message fill a |
119 |
>> bug against the package." |
120 |
>> + die "${FUNCNAME}: check-reqs eclass called but not |
121 |
>> actualy used!" fi |
122 |
>> +} |
123 |
>> |
124 |
>> - if [[ -n "${CHECKREQS_NEED_SLEEP}" ]] ; then |
125 |
>> - echo |
126 |
>> - ewarn "Bad things may happen! You may abort the |
127 |
>> build by pressing ctrl+c in" |
128 |
>> - ewarn "the next 15 seconds." |
129 |
>> - ewarn " " |
130 |
>> - einfo "To make this kind of warning a fatal error, |
131 |
>> add a line to /etc/make.conf" |
132 |
>> - einfo "setting CHECKREQS_ACTION=\"error\". To skip |
133 |
>> build requirements checking," |
134 |
>> - einfo "set CHECKREQS_ACTION=\"ignore\"." |
135 |
>> - epause 15 |
136 |
>> +# @FUNCTION: check-reqs_run |
137 |
>> +# @DESCRIPTION: |
138 |
>> +# Internal function that runs the check based on variable settings. |
139 |
>> +check-reqs_run() { |
140 |
>> + debug-print-function ${FUNCNAME} "$@" |
141 |
>> + |
142 |
>> + # some people are *censored* |
143 |
>> + unset CHECKREQS_FAILED |
144 |
>> + |
145 |
>> + [[ -n ${CHECKREQS_MEMORY} ]]&& \ |
146 |
>> + check-reqs_memory \ |
147 |
>> + ${CHECKREQS_MEMORY} |
148 |
>> + |
149 |
>> + [[ -n ${CHECKREQS_DISK_BUILD} ]]&& \ |
150 |
>> + check-reqs_disk \ |
151 |
>> + "${T}" \ |
152 |
>> + "${CHECKREQS_DISK_BUILD}" |
153 |
> |
154 |
> Why not WORKDIR? |
155 |
I thought workdir is present later than $T plus it was used before and I |
156 |
just kept it. |
157 |
> |
158 |
>> + |
159 |
>> + [[ -n ${CHECKREQS_DISK_USR} ]]&& \ |
160 |
>> + check-reqs_disk \ |
161 |
>> + "${EROOT}/usr" \ |
162 |
>> + "${CHECKREQS_DISK_USR}" |
163 |
>> + |
164 |
>> + [[ -n ${CHECKREQS_DISK_VAR} ]]&& \ |
165 |
>> + check-reqs_disk \ |
166 |
>> + "${EROOT}/var" \ |
167 |
>> + "${CHECKREQS_DISK_VAR}" |
168 |
>> +} |
169 |
> |
170 |
> Also, it may be a good idea to add some kind of generic var for this. |
171 |
> Like: |
172 |
> |
173 |
> CHECKREQS_DISK_INSTALLED=( /usr 1234M /var 2345M ) |
174 |
I dont think that syntax like this would be more readable than the |
175 |
current one. Plus I preffer keeping backcompat the easy way |
176 |
> |
177 |
>> +# @FUNCTION: check-reqs_get_mesg |
178 |
> |
179 |
> Typo. |
180 |
Right |
181 |
> |
182 |
>> +# @DESCRIPTION: |
183 |
>> +# Internal function that returns number in megabites. |
184 |
>> +# Converts from 1G=1024 or 1T=1048576 |
185 |
>> +check-reqs_get_megs() { |
186 |
>> + debug-print-function ${FUNCNAME} "$@" |
187 |
>> + |
188 |
>> + [[ -z ${1} ]]&& die "Usage: ${FUNCNAME} [size]" |
189 |
>> + |
190 |
>> + local unit=${1//[[:digit:]]/} |
191 |
>> + local size=${1//[[:alpha:]]/} |
192 |
> |
193 |
> Seems hacky and poor. What if one uses '1G234'? :P |
194 |
Yeah I will rather ignore it and just expect the users of the eclass are |
195 |
not complete asses. |
196 |
> |
197 |
>> + |
198 |
>> + if [[ ${unit//[[:alpha:]]/} != "" ]]; then |
199 |
>> + die "${FUNCNAME}: Failed to determine units, garbage |
200 |
>> in variables" |
201 |
> |
202 |
> Would that catch anything beside symbols? I guess it would catch '.' as |
203 |
> well. |
204 |
> |
205 |
>> + else |
206 |
>> + # temporary workaround for empty units |
207 |
>> + unit="M" |
208 |
>> + fi |
209 |
>> + |
210 |
>> + case ${unit} in |
211 |
>> + [gG]) echo $((1024 * ${size})) ;; |
212 |
>> + [mM]) echo ${size} ;; |
213 |
>> + [tT]) echo $((1024 * 1024 * ${size})) ;; |
214 |
>> + *) |
215 |
>> + die "${FUNCNAME}: Unknown unit size: ${unit}" |
216 |
> |
217 |
> size unit. |
218 |
> |
219 |
>> + ;; |
220 |
>> + esac |
221 |
>> +} |
222 |
>> + |
223 |
>> +# @FUNCTION: check-reqs_get_numbers |
224 |
> |
225 |
> Why the plural? |
226 |
Why not, tho you are right, removed. |
227 |
> |
228 |
>> +# @DESCRIPTION: |
229 |
>> +# Internal function that returns number without the unit. |
230 |
>> +# Converts from 1G=1 or 150T=150. |
231 |
>> +check-reqs_get_numbers() { |
232 |
>> + debug-print-function ${FUNCNAME} "$@" |
233 |
>> + |
234 |
>> + [[ -z ${1} ]]&& die "Usage: ${FUNCNAME} [size]" |
235 |
>> + |
236 |
>> + local size=${1//[[:alpha:]]/} |
237 |
>> + |
238 |
>> + # warn about un-united variables |
239 |
>> + if [[ ${size} == ${1//[[:alpha:]]/} ]]; then |
240 |
>> + ewarn "QA: Package does not specify unit for the |
241 |
>> size check" |
242 |
>> + ewarn "QA: Assuming Megabytes." |
243 |
>> + ewarn "QA: File bug against the package. It should |
244 |
>> specify it." fi |
245 |
>> |
246 |
>> - if [[ -n "${CHECKREQS_NEED_DIE}" ]] ; then |
247 |
>> - eerror "Bailing out as specified by CHECKREQS_ACTION" |
248 |
>> - die "Build requirements not met" |
249 |
>> + if [[ ${size//[[:digit:]]/} == "" ]]; then |
250 |
>> + echo ${size} |
251 |
>> + else |
252 |
>> + die "${FUNCNAME}: Failed to determine size, garbage |
253 |
>> in variables." fi |
254 |
>> } |
255 |
> |
256 |
> And at this point, I can't stand it anymore. Man, you've transformed |
257 |
> a bad eclass into almost-python.eclass. You should not post diffs when |
258 |
> they don't help with anything; and you should not rewrite eclasses like |
259 |
> that. |
260 |
> |
261 |
> Start from scratch, write check-reqs-r1, make it SIMPLE. |
262 |
> |
263 |
Won't create new function for simple usage like this. |
264 |
|
265 |
If you really consider this similar to python eclass you should look in |
266 |
that again, this is actually quite simple bash compared to it :) |