[pacman-dev] [PATCHv2 1/2] contrib: adding pacsize

[Pierre Neidhardt]

On 14-03-05 18:05:45, Dave Reisner wrote:
> On Wed, Mar 05, 2014 at 09:55:45PM +0100, Pierre Neidhardt wrote:
> > +Display the size of PACKAGES. Duplicates are removed if any. The local database
> > +is queried first; if the package is not found, the sync database is then used
> > +for lookup.
> 
> Duplicates seem rather unexpected given the explanation that follows.
> You're querying either the local DB *or* the sync DB as a fallback. If
> there's duplicates, it's an implementation bug, no?

Left-over from an old version. There cannot be any duplicate indeed.

> > +## All-packages mode.
> > +## We use a dedicated algorithm which is much faster than per-package mode.
> > +## Unfortunately there is no easy way to select packages with this method.
> > +if $opt_all; then
> > +	DBPath="$(awk -F = '/^ *DBPath/{print $2}' @sysconfdir@/pacman.conf 2>/dev/null)"

> What about leading tabs? What about trailing space and tabs? What about
> whitespace between the '=' and the actual value? I'm fairly sure that
> the -d test which follows this fails in pretty much all cases.

Sorry, that was a little quick & dirty. I'll fix that.

> > +	[ ! -d "$DBPath" ] && DBPath="@localstatedir@/lib/pacman"
> > +
> > +	if [ ! -d "$DBPath/local/" ]; then
> > +		error "Could not find local database in $DBPath/local/."
> 
> If pacman.conf contains a DBPath which doesn't exist, the error message
> here will be rather odd, as it'll show the compile time default and not
> the path from pacman.conf.

> > +		exit 1
> > +	fi
> > +
> > +	awk 'BEGIN {
> > +	split("B KiB MiB GiB TiB PiB EiB ZiB YiB", unit)
> > +}
> > +/^%NAME%/ {
> 
> The whole field is %NAME%, there's no need to use a regex here.
> 
> > +	getline
> > +	pkg=$0
> 
> getline pkg
> 
> > +}
> > +/^%SIZE%/ {
> > +	getline
> > +	size = $0
> 
> getline size
> 
> > +	i = 1
> > +	while (size > 2048) {
> > +		size /= 1024
> > +		i++
> > +	}
> > +	printf ("%4d%s %s\n", size, unit[i], pkg)
> > +}' "$DBPath"/local/*/desc | ($opt_sort || cat) | ($opt_total || cat)
> 
> These subshells aren't wanted. You should be using command grouping
> instead.

I thought command grouping did not work in that case (for a reason I couldn't
grasp). Well, I was tricked by my shell. This works in zsh:

  $ ... | {$opt_sort || cat}

while it doesn't in dash or bash. Two errors:
- a space is needed on the inner side of the curly brackets;
- since there is no line break, we need to terminate the statement with a
  semicolon.

Damn zsh.

> > +	exit
> > +fi
> > +
> > +## Per-package mode.
> > +if [ $# -eq 0 ]; then
> > +	error "Missing argument."
> > +	usage "$0"
> > +	exit 1
> > +fi
> > +
> > +if ! command -v pacman >/dev/null 2>&1; then
> 
> I find it very strange that you check for pacman -- the project that
> might distribute this script, but you never check for awk or sort.

Again, a left-over since this was a personal script before.

> > +	error "'pacman' not found."
> > +	exit 1
> > +fi
> > +
> > +{
> > +	## If package is not found locally (-Q), we use the sync database (-S). We
> > +	## use LC_ALL=C to make sure pacman output is not localized.
> > +	buffer=$(LC_ALL=C pacman -Qi "$@" 2>&1 1>&3 3>&- | cut -f2 -d "'")
> > +	[ -n "$buffer" ] && LC_ALL=C pacman -Si $buffer
> 
> Not only are you parsing the output of pacman and the internal format of
> the ALPM db, you're also parsing *error* output from pacman? So much
> groan...

This is the only way I've found to fallback to the sync db without running
calling pacman more than necessary. This is way faster like this.

Of course it's ugly, this is shell scripting. After all, this is the core of
Unix scripting: you pipe and process the output.

We definitely need expac.

> > +} 3>&1 | filter | ($opt_sort || remove_duplicates) | ($opt_total || cat)
> 
> More unnecessary subshells.
> 
> > +
> > +# vim: set noet:
> > -- 
> > 1.9.0
> > 
> > 
> 

-- 
Pierre Neidhardt

Where there are visible vapors, having their prevenance in ignited
carbonaceous materials, there is conflagration.