Hi Mike,
On Sun, Nov 20, 2011 at 1:23 PM, Mike Frysinger <vap...@gentoo.org> wrote:
> On Thursday 03 November 2011 03:28:29 Andy Fleming wrote:
>> +if [ ! "${BUILD_NBUILDS}" ] ; then
>> + BUILD_NBUILDS="1"
>> +fi
>
> : ${BUILD_NBUILDS:=1}
>
>> + if [ ! "${BUILD_DIR}" ] ; then
>> + BUILD_DIR="./build"
>> + fi
>
> : ${BUILD_DIR:=./build}
>
>> + mkdir -p ${BUILD_DIR}/ERR
>
> mkdir -p "${BUILD_DIR}/ERR"
>
>> if [ ! "${BUILD_DIR}" ] ; then
>> BUILD_DIR="."
>> fi
>
> : ${BUILD_DIR:=.}
>
>> + mkdir -p ${output_dir}
>
> mkdir -p "${output_dir}"
>
>> + ${MAKE} clean
>> + find "${output_dir}" -type f -name '*.depend' | xargs rm
>
> why not use distclean and avoid the `find` ? otherwise, this find should be:
> find "${output_dir}" -type f -name '*.depend' -exec rm -f {} +
>
>> + ERR_CNT=$((ERR_CNT + 1))
>
> : $(( ERR_CNT += 1 ))
>
>> - build_target ${t}
>> + if [ "$BUILD_NBUILDS" -gt 1 ] ; then
>> + build_target ${t} &
>> + else
>> + build_target ${t}
>> + fi
>
> you could keep this one line as:
> build_target ${t} &
> [ ${BUILD_NBUILDS} -eq 1 ] && wait
>
>> + TOTAL_CNT=$((TOTAL_CNT + 1))
>
> : $(( TOTAL_CNT += 1 ))
>
>> + CURRENT_COUNT=$((CURRENT_COUNT + 1))
>
> : $(( CURRENT_COUNT += 1 ))
>
>> + # Limit number of parallel builds
>> + if [ ${CURRENT_COUNT} -gt ${BUILD_NBUILDS} ] ; then
>> + CURRENT_COUNT=0;
>> + wait;
>> fi
>
> you don't need those semicolons. also, this is not as good as it should be:
> if you're running 10 jobs in parallel, you fork 10, and then you wait for all
> of them to finish before forking another set of 10.
>
> what you could do is something like:
> JOB_IDX=0
> JOB_IDX_FIRST=0
> BUILD_NBUILDS=1
>
> ... foreach target ... ; do
> build_target &
> pids[$(( JOB_IDX++ ))]=$!
> if [ $(( JOB_IDX - JOB_IDX_FIRST )) -ge ${BUILD_NBUILDS} ] ;
> then
> wait ${pids[$(( JOB_IDX_FIRST++ ))]}
> fi
> done
> wait
>
> this isn't perfect as it assumes the first job launched will always finish
> first,
> but it's a lot closer than the current code.
Since all the jobs are launched at the same time this is not really a
valid assumption is it? IMO on this point it's good enough as it is
for now...
Regards,
Simon
>
>> + for file in `ls ${OUTPUT_PREFIX}/ERR/`; do
>> + ERR_LIST="${ERR_LIST} $file"
>> + done
>
> ERR_LIST=$(ls ${OUTPUT_PREFIX}/ERR/)
>
>> + ERR_CNT=`ls -l ${OUTPUT_PREFIX}/ERR/ | wc | awk '{print $1}'`
>
> you should use -1 instead of -l
> -mike
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
