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

2014-01-17 Thread Sebastian Luther
Am 16.01.2014 01:07, schrieb Tom Wijsman:

 +
 +archive_formats_eapi_3_to_5 = {
 + \.tar.xz:app-arch/tar,
 + \.xz:app-arch/xz-utils,

Shouldn't .tar.xz require both tar and xz-utils? Other entries may
have the same problem.




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

2014-01-17 Thread Sebastian Luther
Am 16.01.2014 22:40, schrieb Tom Wijsman:
 On Thu, 16 Jan 2014 08:03:03 +0100 Sebastian Luther
 sebastianlut...@gmx.de wrote:
 
 Am 16.01.2014 01:07, schrieb Tom Wijsman:
 --- bin/repoman   | 53 
 +
 man/repoman.1 |  4  2 files changed, 57 insertions(+)
 
 diff --git a/bin/repoman b/bin/repoman index d1542e9..9b703dc
 100755 --- a/bin/repoman +++ b/bin/repoman @@ -36,6 +36,9 @@
 pym_path = 
 osp.join(osp.dirname(osp.dirname(osp.realpath(__file__))),
 pym) sys.path.insert(0, pym_path) import portage 
 portage._internal_caller = True + +from portage._sets.profiles
 import PackagesSystemSet +system_set_atoms = 
 PackagesSystemSet(portage.settings.profiles).getAtoms() 
 portage._disable_legacy_globals()
 
 You should be using repoman_settings instead of
 portage.settings.
 
 If I understand correctly, that is this URL?
 
 http://dev.gentoo.org/~zmedico/portage/doc/api/portage.repository.config-module.html

  How do I get the @system set out of that?

portage.settings and repoman_settings are instances of
portage.package.ebuild.config.config. I just looked at the code and
think now that you should keep using PackagesSystemSet to get the
@system atoms. Just turn them into a set afterwards with set(atom.cp
for atom in system_set_atoms).

The reason to use atom.cp is that =cat/foo-1 could be part of that
set and then 'cat/foo in system_set_atoms' would return False.

 
 Considering the later use
 
 Which use?

The if entry not in system_set_atoms line. You're using __contains__
there (with the 'in'). You don't use the additional magic provided by
PackageSet (which is a super class of PackagesSystemSet).

 
 you don't need PackagesSystemSet set here, just use a set.
 
 Okay, thus I need to create some kind of set object here (I don't
 see one in the list of
 http://dev.gentoo.org/~zmedico/portage/doc/api/ though) and then
 specify that it would be the @system set? Which class?
 

Whenever I say 'set' I mean python's builtin set.

 And use atom.cp instead of the atoms.
 
 So, if I understood correctly; using list comprehension, I
 directly transform the getAtoms() to a list of atom.cp's... Okay,
 good idea.
 
 try: @@ -300,6 +303,7 @@ qahelp = { inherit.missing: Ebuild
 uses functions from an eclass but does not inherit it,
 inherit.unused: Ebuild inherits an eclass but does not use
 it, java.eclassesnotused: With virtual/jdk in DEPEND you
 must inherit a java eclass, +  unpack.DEPEND.missing: A rare
 archive format was used in SRC_URI, but its package to unpack
 it is missing in DEPEND., wxwidgets.eclassnotused: Ebuild
 DEPENDs on x11-libs/wxGTK without inheriting wxwidgets.eclass,
 KEYWORDS.dropped: Ebuilds that appear to have dropped
 KEYWORDS for some arch, KEYWORDS.missing: Ebuilds that have
 a missing or empty KEYWORDS variable, @@ -399,6 +403,7 @@
 qawarnings = set(( metadata.warning, portage.internal, 
 repo.eapi.deprecated, +unpack.DEPEND.missing, 
 usage.obsolete, upstream.workaround, LIVEVCS.stable, @@
 -479,6 +484,25 @@ ruby_deprecated = frozenset([ 
 ruby_targets_ree18, ])
 
 +# TODO: Add functionality to support checking for deb2targz
 on platforms where +#   GNU binutils is absent; see PMS 5,
 section 11.3.3.13. +archive_formats = { +
 \.7[zZ]:app-arch/p7zip, +
 \.(bz2?|tbz2):app-arch/bzip2, + \.jar:app-arch/unzip, +
 \.(LH[aA]|lha|lzh):app-arch/lha, +
 \.lzma:app-arch/lzma-utils, +
 \.(rar|RAR):app-arch/unrar, +
 \.(tar(\.(bz2?|gz|Z))?|tbz2|t[bg]z)?:app-arch/tar, +
 \.(gz|tar\.Z|t[bg]z|[zZ]):app-arch/gzip, +
 \.(zip|ZIP):app-arch/unzip, +} + 
 +archive_formats_eapi_3_to_5 = { +  \.tar.xz:app-arch/tar, +
 \.xz:app-arch/xz-utils, +} + metadata_xml_encoding =
 'UTF-8' metadata_xml_declaration = '?xml version=1.0
 encoding=%s?' % \ (metadata_xml_encoding,) @@ -1559,6
 +1583,7 @@ for x in effective_scanlist: fetchlist_dict =
 portage.FetchlistDict(checkdir, repoman_settings, portdb)
 myfiles_all = [] src_uri_error = False +needed_unpack_depends
 = {} for mykey in fetchlist_dict: try: 
 myfiles_all.extend(fetchlist_dict[mykey]) @@ -1573,7 +1598,22
 @@ for x in effective_scanlist: stats[SRC_URI.syntax] += 1 
 fails[SRC_URI.syntax].append( %s.ebuild SRC_URI: %s % 
 (mykey, e)) + + # Compare each SRC_URI entry against 
 archive_formats; if one of the +# extensions match, we
 remember which archive depends are needed to +  # check them
 later on. + needed_unpack_depends[mykey] = [] + for
 file_extension in archive_formats or \ +
 ((re.match('[345]$',
 eapi) is not None) \
 
 Use portage.eapi for the line above.
 
 Why? 'eapi' is the EAPI of the ebuild, what is wrong with that?

What I want you to do is to change (re.match('[345]$', eapi) into
something like: eapi_has_xz_unpack(eapi). the function
eapi_has_xz_unpack needs to be written. It should be part of portage.eapi.

 
 You may have to add a new function to portage.eapi.
 
 What would the purpose of that function 

[gentoo-portage-dev] [PATCH 2/3 v2] Have repoman check that a package directory contains at least one ebuild (bug #245305).

2014-01-17 Thread Tom Wijsman
---
 bin/repoman   | 8 
 man/repoman.1 | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/bin/repoman b/bin/repoman
index d1542e9..44f3d3d 100755
--- a/bin/repoman
+++ b/bin/repoman
@@ -326,6 +326,7 @@ qahelp = {
SRC_URI.mirror: A uri listed in profiles/thirdpartymirrors is found 
in SRC_URI,
ebuild.syntax: Error generating cache entry for ebuild; typically 
caused by ebuild syntax error or digest verification failure,
ebuild.output: A simple sourcing of the ebuild produces output; this 
breaks ebuild policy.,
+   ebuild.missing: A package directory must at least contain one ebuild 
or be treecleaned.,
ebuild.nesteddie: Placing 'die' inside ( ) prints an error, but 
doesn't stop the ebuild.,
variable.invalidchar: A variable contains an invalid character that 
is not part of the ASCII character set,
variable.readonly: Assigning a readonly variable,
@@ -1442,6 +1443,13 @@ for x in effective_scanlist:
can_force = False
continue
 
+   if not ebuildlist:
+   stats[ebuild.missing] += 1
+   fails[ebuild.missing].append(%s must at least contain one  
% x + \
+   ebuild or be treecleaned.)
+   can_force = False
+   continue
+
# Sort ebuilds in ascending order for the KEYWORDS.dropped check.
ebuildlist = sorted(pkgs.values())
ebuildlist = [pkg.pf for pkg in ebuildlist]
diff --git a/man/repoman.1 b/man/repoman.1
index a78f94e..6315ea9 100644
--- a/man/repoman.1
+++ b/man/repoman.1
@@ -301,6 +301,9 @@ Ebuilds that exist but have not been added to cvs
 .B ebuild.output
 A simple sourcing of the ebuild produces output; this breaks ebuild policy.
 .TP
+.B ebuild.missing
+A package directory must at least contain one ebuild or be treecleaned.
+.TP
 .B ebuild.patches
 PATCHES variable should be a bash array to ensure white space safety
 .TP
-- 
1.8.5.2




Re: [gentoo-portage-dev] [PATCH 2/3 v2] Have repoman check that a package directory contains at least one ebuild (bug #245305).

2014-01-17 Thread Jesus Rivero (Neurogeek)
On Fri, Jan 17, 2014 at 4:36 PM, Tom Wijsman tom...@gentoo.org wrote:

 ---
  bin/repoman   | 8 
  man/repoman.1 | 3 +++
  2 files changed, 11 insertions(+)

 diff --git a/bin/repoman b/bin/repoman
 index d1542e9..44f3d3d 100755
 --- a/bin/repoman
 +++ b/bin/repoman
 @@ -326,6 +326,7 @@ qahelp = {
 SRC_URI.mirror: A uri listed in profiles/thirdpartymirrors is
 found in SRC_URI,
 ebuild.syntax: Error generating cache entry for ebuild;
 typically caused by ebuild syntax error or digest verification failure,
 ebuild.output: A simple sourcing of the ebuild produces output;
 this breaks ebuild policy.,
 +   ebuild.missing: A package directory must at least contain one
 ebuild or be treecleaned.,
 ebuild.nesteddie: Placing 'die' inside ( ) prints an error, but
 doesn't stop the ebuild.,
 variable.invalidchar: A variable contains an invalid character
 that is not part of the ASCII character set,
 variable.readonly: Assigning a readonly variable,
 @@ -1442,6 +1443,13 @@ for x in effective_scanlist:
 can_force = False
 continue

 +   if not ebuildlist:
 +   stats[ebuild.missing] += 1
 +   fails[ebuild.missing].append(%s must at least contain
 one  % x + \
 +   ebuild or be treecleaned.)
 +   can_force = False
 +   continue
 +
 # Sort ebuilds in ascending order for the KEYWORDS.dropped check.
 ebuildlist = sorted(pkgs.values())
 ebuildlist = [pkg.pf for pkg in ebuildlist]
 diff --git a/man/repoman.1 b/man/repoman.1
 index a78f94e..6315ea9 100644
 --- a/man/repoman.1
 +++ b/man/repoman.1
 @@ -301,6 +301,9 @@ Ebuilds that exist but have not been added to cvs
  .B ebuild.output
  A simple sourcing of the ebuild produces output; this breaks ebuild
 policy.
  .TP
 +.B ebuild.missing
 +A package directory must at least contain one ebuild or be treecleaned.
 +.TP
  .B ebuild.patches
  PATCHES variable should be a bash array to ensure white space safety
  .TP
 --
 1.8.5.2



Looks fine.

-- 
Jesus Rivero (Neurogeek)
Gentoo Developer


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

2014-01-17 Thread Tom Wijsman
On Fri, 17 Jan 2014 09:35:58 +0100
Sebastian Luther sebastianlut...@gmx.de wrote:

 The if entry not in system_set_atoms line. You're using __contains__
 there (with the 'in'). You don't use the additional magic provided by
 PackageSet (which is a super class of PackagesSystemSet).

This is __contains__, which does what I want as far as I can see:
http://dev.gentoo.org/~zmedico/portage/doc/api/portage._sets.base-pysrc.html#PackageSet.__contains__
What is there wrong with this?

As for the additional magic, do you mean containsCPV? Looking at it:
http://dev.gentoo.org/~zmedico/portage/doc/api/portage._sets.base-pysrc.html#PackageSet.containsCPV
It seems more complex than is necessary, is there a benefit to this?

Sorry, I don't understand what is meant here; all the rest of your
feedback is clear and has been implemented and is in the v2 I plan to
send to the mailing list soon.

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : tom...@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D


signature.asc
Description: PGP signature


[gentoo-portage-dev] When must we write tests, when is it optional?

2014-01-17 Thread Tom Wijsman
Hello

Looking back on the repoman patches from both me and creffett, we
haven't written tests; it's on my plan to still do that, if possible.

This makes me wonder: Should writing tests be a requirement? When?

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : tom...@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D


signature.asc
Description: PGP signature


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

2014-01-17 Thread Sebastian Luther
Am 18.01.2014 00:00, schrieb Tom Wijsman:
 On Fri, 17 Jan 2014 09:35:58 +0100
 Sebastian Luther sebastianlut...@gmx.de wrote:
 
 The if entry not in system_set_atoms line. You're using __contains__
 there (with the 'in'). You don't use the additional magic provided by
 PackageSet (which is a super class of PackagesSystemSet).
 
 This is __contains__, which does what I want as far as I can see:
 http://dev.gentoo.org/~zmedico/portage/doc/api/portage._sets.base-pysrc.html#PackageSet.__contains__
 What is there wrong with this?

There's nothing wrong with that. It's just that the builtin set type
supports that too.

 
 As for the additional magic, do you mean containsCPV? Looking at it:
 http://dev.gentoo.org/~zmedico/portage/doc/api/portage._sets.base-pysrc.html#PackageSet.containsCPV
 It seems more complex than is necessary, is there a benefit to this?

I thought more about iterAtomsForPackage, which is used in lots of
places in the resolver.

The comment was about the fact that you're using a more complicated
selfmade class instead of a builtin type, even if the builtin type would
suffice. The comment made more sense when I was still suggesting to not
use PackageSystemSet at all.



Re: [gentoo-portage-dev] When must we write tests, when is it optional?

2014-01-17 Thread Mike Frysinger
On Saturday 18 January 2014 01:40:04 Mike Frysinger wrote:
 On Friday 17 January 2014 19:10:56 Tom Wijsman wrote:
  Looking back on the repoman patches from both me and creffett, we
  haven't written tests; it's on my plan to still do that, if possible.
  
  This makes me wonder: Should writing tests be a requirement? When?
 
 i'm not sure if it should be a hard requirement, but it is strongly
 encouraged

i would even encourage reviewers to inquire about tests, especially when the 
change is not entirely obvious
-mike


signature.asc
Description: This is a digitally signed message part.