Re: [gentoo-portage-dev] [PATCH 1/3 v2] Have repoman check if the packages to unpack rare archive formats from SRC_URI are present in DEPEND (bug #205909).

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: Text/Plain, Size: 1977 bytes --]

On Friday 17 January 2014 18:03:57 Tom Wijsman wrote:
> ---

please shorten your commit summary and move more content to the body

> +getNonSystemArchiveDepends.archivers = {

it is super weird to attach to the object like this.  some might even say 
wrong.  move it into the class definition.

def getNonSystemArchiveDepends(fetchlist, eapi):
	...

	ARCHIVERS = {
		...
	}

> +	re.compile('.*\.7[zZ]$'):"app-arch/p7zip",

regexes should always use raw strings.  there should also be a space after the 
colon in dicts.  so you want:

	re.compile(r'.*\.7[zZ]$'): 'app-arch/p7zip',

> +	re.compile('.*\.lzma$'):"app-arch/lzma-utils",

xz-utils, not lzma-utils

> +	re.compile('.*\.(bz2?|tbz2)$'):"app-arch/bzip2",
> +	re.compile('.*\.(tar(\.(bz2?|gz|Z))?|tbz2|t[bg]z)?$'):"app-arch/tar",
> +	re.compile('.*\.(gz|tar\.Z|t[bg]z|[zZ])$'):"app-arch/gzip",
> +	re.compile('.*\.tar.xz$'):"app-arch/tar",

requiring people list gzip, tar, and bzip2 is a significant policy change 
(which i'm inclined to say is wrong).  it needs discussion on the dev mailing 
list first.

> +def checkArchiveDepends(atoms, catpkg, relative_path, \
> +	system_set_atoms, needed_unpack_depends, stats, fails):

you don't need the \ there because you have paren already to gather things.

> +	for entry in needed_unpack_depends[catpkg]:
> +		if entry not in [atom.cp for atom in atoms if atom != "||"]:
> +			stats['unpack.' + mytype + '.missing'] += 1
> +			fails['unpack.' + mytype + '.missing'].append( \
> +				relative_path + ": %s is missing in %s" % \
> +				(entry, mytype))

i know existing style doesn't follow it, but imo we should avoid string 
concatenation.  it makes the code harder to read imo.

	key = 'unpack.%s.missing' % mytype
	stats[key] += 1
	fails[key].append(...)

> +def eapi_has_xz_utils(eapi):
> +	return eapi not in ("0", "1", "2")

i would use "eapi_supports_xz_archives" unless there's a pre-existing style
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]