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).

[Alec Warner <antarus@gentoo.org>]

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

On Wed, Jan 15, 2014 at 4:07 PM, Tom Wijsman <tomwij@gentoo.org> wrote:

> ---
>  bin/repoman   | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  man/repoman.1 |  4 ++++
>  2 files changed, 57 insertions(+)
>
>
I urge you to not author new checks like this. /usr/bin/repoman is already
a mess.

Write these checks as functions, put them in a different file, and just
call them from the giant messy loop. At least that way the checks are self
contained (great for avoiding things like variable re-use or shadowing).

-A



> 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()
>
>  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) \
> +                               and file_extension in
> archive_formats_eapi_3_to_5):
> +                       for entry in fetchlist_dict[mykey]:
> +                               if re.match('.*%s$' % file_extension,
> entry) is not None:
> +                                       format =
> archive_formats[file_extension]
> +
> +                                       if format not in
> needed_unpack_depends[mykey]:
> +
> needed_unpack_depends[mykey].append(format)
>         del fetchlist_dict
> +
>         if not src_uri_error:
>                 # This test can produce false positives if SRC_URI could
> not
>                 # be parsed for one or more ebuilds. There's no point in
> @@ -2010,6 +2050,17 @@ for x in effective_scanlist:
>                                 atoms = None
>                                 badsyntax.append(str(e))
>
> +                       if atoms and mytype == 'DEPEND':
> +                               # We check whether the needed archive
> dependencies are present
> +                               # in DEPEND, which were determined from
> SRC_URI.
> +                               for entry in needed_unpack_depends[catdir
> + '/' + y]:
> +                                       if entry not in system_set_atoms
> and 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))
> +
>                         if atoms and mytype.endswith("DEPEND"):
>                                 if runtime and \
>                                         "test?" in mydepstr.split():
> @@ -2384,6 +2435,8 @@ for x in effective_scanlist:
>                                 "%s/metadata.xml: unused local
> USE-description: '%s'" % \
>                                 (x, myflag))
>
> +       del needed_unpack_depends
> +
>  if options.if_modified == "y" and len(effective_scanlist) < 1:
>         logging.warn("--if-modified is enabled, but no modified packages
> were found!")
>
> diff --git a/man/repoman.1 b/man/repoman.1
> index a78f94e..e739d56 100644
> --- a/man/repoman.1
> +++ b/man/repoman.1
> @@ -334,6 +334,10 @@ Ebuild inherits a deprecated eclass
>  With virtual/jdk in DEPEND you must inherit a java eclass. Refer to
>  \fIhttp://www.gentoo.org/proj/en/java/java\-devel.xml\fR for more
> information.
>  .TP
> +.B unpack.DEPEND.missing
> +A rare archive format was used in SRC_URI, but its package to unpack it is
> +missing in DEPEND.
> +TP
>  .B manifest.bad
>  Manifest has missing or incorrect digests
>  .TP
> --
> 1.8.5.2
>
>
>

[-- Attachment #2: Type: text/html, Size: 9252 bytes --]