Re: [devel] [git update] packages/hasher: heads/master

["Dmitry V. Levin" <ldv@altlinux.org>]

[-- Attachment #1: Type: text/plain, Size: 2026 bytes --]

On Fri, Aug 10, 2007 at 08:29:33PM +0400, Alexey M. Tourbin wrote:
> Update of /people/at/packages/hasher.git
[...]
>     On the "install ... && Verbose ... || Fatal ..." clauses.  I think this
>     is overkill.

Мне важно не только видеть причину сбоя, но и реакцию программы на сбой.
Возможно, диагностика от install и достаточна.  Решение принимается
субъективно в каждом конкретном случае.

>     On the "if shell_function ...; then ...' clauses.  This is evil.  Don't use
>     shell function in the "if" conditions, because "sh -e" mode is disabled for
>     such calls.  You can no longer expect that something "just fails".

То же самое касается и shell_function && on_success || on_failure.

>     On the "cat file |while read ...; grep || echo >bad; .. done' clauses.
>     I replaced them with "while ... done <file".

Если можно обойтись без подпроцессов, то так и надо делать.

> +	rm -rf cache/contents/list.new
> +	mkdir -p $verbose cache/contents/list.new

Здесь и далее, "rm -rf" с последующим "mkdir -p" на один и тот же каталог --
это неконсистентно; "-p" сбивает с толку читающего код.

> +	rm -rf cache/contents/tmp
> +	mkdir -p cache/contents/tmp
> +	local contents=cache/contents/tmp/contents
[...]
> +	contents_index_bin=chroot/.host/contents_index_bin
[...]
> +	cat <"$contents.bin" >"$contents_index_bin"
> +	rm -rf cache/contents/tmp

Здесь можно заменить cat на mv.

[...]
> +	rm -rf cache/contents/{index,list}
> +	install -p -m644 $verbose chroot/.host/contents_index_bin cache/contents/contents_index_bin
> +	install -p -m644 $verbose chroot/.host/contents_index_all.gz cache/contents/contents_index_all.gz

Тогда уж ln.

> +	contents_index_bin=chroot/.host/contents_index_bin
> +	contents_index_all=chroot/.host/contents_index_all.gz
> +	install -p -m644 $verbose cache/contents/contents_index_bin "$contents_index_bin"
> +	install -p -m644 $verbose cache/contents/contents_index_all.gz "$contents_index_all"

И здесь можно ln.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]