[Buildroot] [PATCH 8/9] manual: update {deprecated, package}-list.txt

Samuel Martin s.martin49 at gmail.com
Mon Feb 25 21:22:39 UTC 2013


Hi Arnout,

2013/2/18 Arnout Vandecappelle <arnout at mind.be>:
>  Hi Samuel,
>
>
> On 13/02/13 23:59, Samuel Martin wrote:
>>
>> * add the gen-manual-lists.py script generating the target and host
>>    package tables, and the deprecated stuff list as well. These tables
>>    and lists are generated parsing the Config.in files;
>> * add a make target to update these lists in the manual;
>> * update the these appendices of the manual.
>
>
>  The first two should be a separate patch from the third one. Although,
> admittedly, it was pretty convenient to review all of them together.
I hesitated to do so ;)

>
>  Even though I have a lot of comments, this patch should be included in
> -rc2, otherwise the package list in the manual is inconsistent.
>
>
>
>>
>> Signed-off-by: Samuel Martin <s.martin49 at gmail.com>
>> ---
>>   docs/manual/appendix.txt            |  13 +-
>>   docs/manual/deprecated-list.txt     |  81 ++-
>>   docs/manual/manual.mk               |   7 +
>>   docs/manual/package-list.txt        | 948
>> ++++++++++++++++++++++++++++++++++++
>>   docs/manual/pkg-list.txt            | 870
>> ---------------------------------
>
>
>  Any particular reason to rename it?
Using plain noun in file names, keeping consistency with the rest of
the doc source file names.

BTW, restrict the generated stuff to the tables themselves implies
generating 3 files.
So, automating this generation will certainly lead to rename few files.

>
>
>>   support/scripts/gen-manual-lists.py | 237 +++++++++
>>   6 files changed, 1233 insertions(+), 923 deletions(-)
>>   create mode 100644 docs/manual/package-list.txt
>>   delete mode 100644 docs/manual/pkg-list.txt
>>   create mode 100755 support/scripts/gen-manual-lists.py
>>
>> diff --git a/docs/manual/appendix.txt b/docs/manual/appendix.txt
>> index ef34169..37548f7 100644
>> --- a/docs/manual/appendix.txt
>> +++ b/docs/manual/appendix.txt
>> @@ -6,15 +6,8 @@ Appendix
>>
>>   include::makedev-syntax.txt[]
>>
>> -[[package-list]]
>> -Available packages
>> -------------------
>> -// docs/manaual/pkg-list.txt is generated using the following command:
>> -// $ git grep -E '\((autotools|cmake|generic)-package\)' package/ |  \
>> -//     cut -d':' -f1 | grep '\.mk$' | \
>> -//     sed -e 's;.*\?/\(.*\?\).mk;* \1;' | \
>> -//     sort > docs/manual/pkg-list.txt
>> -
>> -include::pkg-list.txt[]
>> +// autogenerated
>> +include::package-list.txt[]
>>
>> +// autogenerated
>>   include::deprecated-list.txt[]
>> diff --git a/docs/manual/deprecated-list.txt
>> b/docs/manual/deprecated-list.txt
>> index 6dc87a4..3468219 100644
>> --- a/docs/manual/deprecated-list.txt
>> +++ b/docs/manual/deprecated-list.txt
>> @@ -1,46 +1,41 @@
>> -// -*- mode:doc -*- ;
>> -
>> -[[deprecated]]
>> -Deprecated list
>> ----------------
>> -
>> -The following stuff are marked as _deprecated_ in Buildroot due to
>> -their status either too old or unmaintained.
>
>
>  It's a pity that this part is gone. I would by the way move this to
> appendix.txt: that's cleaner than putting part of the manual text in the
> python script. Same for the text in package-list.txt.

Good catch! I didn't notice this was dropped.
BTW, for the package list, there was no text, just a commented command
line used to generate the list, which is obsolete with this patch...

>
> [snip]
>
>> diff --git a/docs/manual/manual.mk b/docs/manual/manual.mk
>> index aa20534..dd07b4e 100644
>> --- a/docs/manual/manual.mk
>> +++ b/docs/manual/manual.mk
>> @@ -24,6 +24,13 @@ $$(O)/docs/$(1)/$(1).$(4): docs/$(1)/$(1).txt $$($(call
>> UPPERCASE,$(1))_SOURCES)
>>           -D $$(@D) $$<
>>   endef
>>
>> +manual-update-lists:
>> +       @$(call MESSAGE,"Updating the manual lists...")
>> +       $(Q)BR2_DEFCONFIG="" srctree=$(TOPDIR) \
>> +               $(TOPDIR)/support/scripts/gen-manual-lists.py \
>> +               --package-file $(TOPDIR)/docs/manual/package-list.txt \
>> +               --deprecated-file
>> $(TOPDIR)/docs/manual/deprecated-list.txt
>> +
>
>
>  This new goal should be added to .PHONY.
done.

>
>
>>
>> ################################################################################
>>   # GENDOC -- generates the make targets needed to build asciidoc
>> documentation.
>>   #
>> diff --git a/docs/manual/package-list.txt b/docs/manual/package-list.txt
>> new file mode 100644
>> index 0000000..79d4ec6
>> --- /dev/null
>> +++ b/docs/manual/package-list.txt
>> @@ -0,0 +1,948 @@
>> +//
>> +// Automatically generated list for Buildroot manual.
>> +// Buildroot 2013.02-00794-g199d6d6-dirty
>> +// Generation date: 2013-02-11 22:24:44.739087
>> +//
>> +
>> +[[package-list]]
>> +Package Selection for the target
>> +--------------------------------
>> +
>> +[width="90%",cols="^1,4",options="header"]
>> +|===================================================
>> +| Packages                       | Package Selection for the target ->
>> ...
>
>
>  Nice! Maybe even better if this stands out a bit from the rest, perhaps:
ok, I'll add a blank line before and after the table content.

>
> <| Packages                     | Package Selection for the target -> ...
>>
>> +| acl                            | . -> System tools
>> +| acpid                          | . -> Hardware handling
>> +| alsa-lib                       | . -> Libraries -> Audio/Sound
>> +| alsa-utils                     | . -> Audio and video applications
>> +| alsamixergui                   | . -> Graphic libraries and
>> applications (graphic/text)
>> +| applewmproto                   | . -> Graphic libraries and
>> applications (graphic/text) -> X11R7 X protocols
>> +| appres                         | . -> Graphic libraries and
>> applications (graphic/text) -> X11R7 Applications
>> +| apr                            | . -> Libraries -> Other
>> +| apr-util                       | . -> Libraries -> Other
>> +| argp-standalone                | . -> Libraries -> Other
>> +| argus                          | . -> Networking applications
>> +| arptables                      | . -> Networking applications
>> +| at                             | . -> Shell and utilities
>> +| atk                            | . -> Libraries -> Graphics
>> +| attr                           | . -> System tools
>> +| audiofile                      | . -> Libraries -> Audio/Sound
>> +| aumix                          | . -> Audio and video applications
>> +| autoconf *(deprecated)*        | . -> Development tools
>> +| automake *(deprecated)*        | . -> Development tools
>
>
>  Is it intentional to keep the deprecated ones in this list?

Yes, shall we drop them from the list?
Though I think deprecated features should not be documented, I have no
strong opinion about this specific point.

Someone may think it is redundant with the "Deprecated" appendix.
But for packages, it is also a way to tell to the users, they should
either not use packages marked as deprecated,
or do some work to maintain it and get it back in the official
supported package list...

>
> [snip]
>
>
>> diff --git a/support/scripts/gen-manual-lists.py
>> b/support/scripts/gen-manual-lists.py
>> new file mode 100755
>> index 0000000..f2a2dc4
>> --- /dev/null
>> +++ b/support/scripts/gen-manual-lists.py
>> @@ -0,0 +1,237 @@
>> +#!/usr/bin/env python2
>> +##
>> +## gen-manual-lists.py
>> +##
>> +## This script generates the following Buildroot manual appendices:
>> +##  - the package tables (one for the target, the other for host tools);
>> +##  - the deprecated items.
>> +##
>> +## Author(s):
>> +##  - Samuel Martin <s.martin49 at gmail.com>
>> +##
>> +## Copyright (C) 2013 Samuel Martin
>> +##
>> +## This program is free software; you can redistribute it and/or modify
>> +## it under the terms of the GNU General Public License as published by
>> +## the Free Software Foundation; either version 2 of the License, or
>> +## (at your option) any later version.
>> +##
>> +## This program is distributed in the hope that it will be useful,
>> +## but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +## GNU General Public License for more details.
>> +##
>> +## You should have received a copy of the GNU General Public License
>> +## along with this program; if not, write to the Free Software
>> +## Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
>> USA
>> +##
>> +
>> +## Note about python2.
>> +##
>> +## This script can currently only be run using python2 interpreter due to
>> +## its kconfiglib dependency (which is not yet python3 friendly).
>> +
>> +from __future__ import print_function
>> +from __future__ import unicode_literals
>
>
>  These future things only exist in recent pythons, and we don't do any
> checks for a specific python version... That said, this script isn't
> supposed to be run by a normal user so I guess it doesn't really matter.
Indeed, it does not really matter since the kconfiglib package is not
python3-friendly, so it could have been write in pure python2...

>
>
>> +
>> +import os
>> +import re
>> +import sys
>> +import datetime
>> +from argparse import ArgumentParser
>> +
>> +try:
>> +    import kconfiglib
>> +except ImportError:
>> +    message = """
>> +Could not find the module 'kconfiglib' in the PYTHONPATH:
>> +"""
>> +    message += "\n".join(["  {0}".format(path) for path in sys.path])
>> +    message += """
>> +
>> +Make sure the Kconfiglib directory is in the PYTHONPATH, then relaunch
>> the script.
>> +
>> +You can get kconfiglib from:
>> +  https://github.com/ulfalizer/Kconfiglib
>
>
>  Can we not just dump a copy of kconfiglib in the scripts directory? Or is
> it more than one file?
I think we can, it's just one *.py file. I'll do it.

>
>
>> +
>> +
>> +"""
>> +    sys.stderr.write(message)
>> +    raise
>> +
>> +
>> +_LIST_FORMAT = """\
>
>
>  This symbol would be more appropriate as an attribute of the Buildroot
> class.
>
>
>> +//
>> +// Automatically generated list for Buildroot manual.
>> +// Buildroot {br_version_full}
>> +// Generation date: {gen_date}
>> +//
>> +
>> +[[{section_anchor}]]
>> +{section_title}
>> +{section_level}
>
>
>  As I briefly mentioned before: I think it's better to just generate the
> table with the script, and keep the surrounding text and title in the
> manually written asciidoc files. That means you'll need three files, but
> there's no issue with that, is there.
ok, I'll put all the surrounding text in the appendix.txt file.

>
>
>> +
>> +{table}
>> +"""
>> +
>> +
>> +def format_asciidoc_table(root, items, get_label, filter=lambda x: True,
>> sort=True):
>
>
>  I would prefer using different names than standard python functions. So
> filterfunc instead of filter. sort is OK because it only exists as an
> attribute of list objects.
ok.

>
>
>> +    def _format_entry(label, parents):
>> +        return "| {0:<30} | {1}\n".format(label, " -> ".join(parents))
>> +    def _get_parents(root, item):
>> +        parent = item.get_parent()
>> +        parents = []
>> +        while parent and parent != root:
>> +            if parent.is_menu():
>
>
>  I guess the parent can be either a menu or a choice. Seems a good idea to
> me to keep choices in the path as well.
done.

>
>
>> +                parents += [parent.get_title()]
>> +            parent = parent.get_parent()
>> +        parents.reverse()
>> +        return parents
>
>
>  An empty line here would be nice.
>
>
>> +    lines = []
>> +    for item in items:
>> +        if not filter(item):
>> +            continue
>> +        if not item.is_symbol() or not item.prompts:
>> +            continue
>> +        loc = [] if root is None else ["."]
>> +        loc += _get_parents(root, item)
>> +        lines += [_format_entry(get_label(item), loc)]
>
>
>  lines.append(...) would be more appropriate.
>
>
>> +    if sort:
>> +        lines.sort(key=lambda x:x.lower())
>> +    if root:
>> +        loc_label = _get_parents(None, root) + [root.get_title(), "..."]
>> +    else:
>> +        loc_label = ["Location"]
>> +    lines.insert(0, _format_entry("Packages", loc_label))
>> +    table = "[width=\"90%\",cols=\"^1,4\",options=\"header\"]\n"
>> +    table += "|===================================================\n"
>> +    table += "".join(lines)
>> +    table += "|===================================================\n"
>> +    return table
>> +
>> +def get_symbol_subset(items, filter):
>
>
>  Same remark about filter here.
>
>
>> +    subset = []
>> +    for item in items:
>> +        if item.is_symbol():
>> +            if not item.prompts:
>> +                continue
>> +            if not filter(item):
>> +                continue
>> +            subset += [item]
>
>
>  subset.append(item)
>
>
>> +        elif item.is_menu() or item.is_choice():
>> +            subset += get_symbol_subset(item.get_items(), filter)
>> +    return subset
>
>
>  This function is an ideal candidate for an iterator. But iterators are a
> bit tricky to understand so I don't mind keeping it as it is.
done.

>
>
>> +
>> +
>> +class Buildroot:
>> +    root_config = "Config.in"
>> +    package_dirname = "package"
>> +    package_prefixes = ["BR2_PACKAGE_", "BR2_PACKAGE_HOST_"]
>> +    deprecated_symbol = "BR2_DEPRECATED"
>> +
>> +    def __init__(self):
>> +        self.base_dir = os.environ.get("srctree")
>
>
>  Why not use the TOPDIR variable?
Indeed, it'd be better.
BTW, the kconfiglib.Config class expects (and requires) a "srctree"
variable in the environment.

>
>
>> +        self.package_dir = os.path.join(self.base_dir,
>> self.package_dirname)
>> +        self.config = kconfiglib.Config(os.path.join(self.base_dir,
>> self.root_config))
>> +        self.package_symbols = []
>> +        self._fill_package_symbols()
>> +        self._deprecated = self.config.get_symbol(self.deprecated_symbol)
>> +        self.br_version_full = os.environ.get("BR2_VERSION_FULL")
>> +        self.generation_date = str(datetime.datetime.utcnow())
>> +
>> +    def _get_package_symbol(self, package_name):
>> +        pkg_symbol = re.sub("[-+.]", "_", package_name)
>> +        pkg_symbol = pkg_symbol.upper()
>> +        pkg_symbols = [prefix + pkg_symbol for prefix in
>> self.package_prefixes]
>> +        return pkg_symbols
>> +
>> +    def _fill_package_symbols(self):
>> +        for root, _, files in os.walk(self.package_dir):
>> +            pkg_name = os.path.basename(root)
>> +            for file_ in files:
>> +                if not file_.endswith(".mk") or not pkg_name ==
>> file_[:-3]:
>> +                    continue
>> +                self.package_symbols +=
>> self._get_package_symbol(pkg_name)
>
>
>  I don't really like this. Wouldn't it be possible to construct the package
> name from the location of the Config.in where it is defined?
Me neither, but the problem is that few package symbols are not
defined in their own package directory :(
For example, libjpeg and jpeg-turbo symbols are both defined in
package/jpeg/Config.in;
so just looking at the location of the Config.in leads to missing both
the libjpeg and jpeg-turbo packages.

Another problem is that the kconfig symbol/package name tuple is not a
bijection, e.g.:
from the kconfig symbol, we get several package names (and this number
increase a lot when the case is not ignored):
BR2_PACKAGE_XLIB_LIBXAU => xlib-libxau | xlib_libxau | xlib-libXau |
xlib_libXau | ...

from the package name, we easy get the unique kconfig symbol:
xlib_libXau => BR2_PACKAGE_XLIB_LIBXAU

So, it is more expensive to get the package name from the kconfig
symbol than getting the symbol from the package name.

Note that the actual package check would be grepping the *.mk files
for '\$(eval \$(\(host-\)\?\(generic\|autotools\|cmake\)-package))'.
So, I made a choice. With this compromise, no package is missed, but
the "virtual" packages (that actually are choices) are also catched.


> And check for it directly in _is_package instead of pre-constructing the list?
done.

>
>
>> +
>> +    def _is_deprecated(self, symbol):
>> +        return self._deprecated in symbol.get_referenced_symbols()
>> +
>> +    def _is_from_package(self, symbol):
>> +        if hasattr(symbol, 'get_def_locations'):
>> +            def_loc = symbol.get_def_locations()[0][0]
>> +        elif hasattr(symbol, 'get_location'):
>> +            def_loc = symbol.get_location()[0]
>> +        else:
>> +            return False
>> +        return self.package_dir in def_loc
>
>
>  This function isn't used, is it?
Indeed.

>
>
>> +
>> +    def _is_package(self, symbol):
>> +        # return symbol.get_name().startswith(self.package_prefixes[0])
>> +        return symbol.get_name() in self.package_symbols
>> +        pkg_name =
>> os.path.basename(os.path.dirname(symbol.get_def_locations()[0][0]))
>> +        return symbol.get_name() in self._get_package_symbol(pkg_name)
>
>
>  The final commit shouldn't contain three different versions :-)
Arf... I forgot to cleanup before posting!

>
>
>> +
>> +    def _get_symbol_label(self, item):
>> +        label = item.prompts[0][0]
>> +        if self._is_deprecated(item):
>> +            label += " *(deprecated)*"
>> +        return label
>> +
>> +    def _format_list(self, root, items, section_anchor, section_title):
>> +        table = format_asciidoc_table(root, items,
>> get_label=self._get_symbol_label)
>> +        content = _LIST_FORMAT.format(
>> +            br_version_full=self.br_version_full,
>> +            gen_date=self.generation_date,
>> +            section_anchor=section_anchor,
>> +            section_title=section_title,
>> +            section_level=re.sub(".", "-", section_title),
>> +            table=table
>> +            )
>> +        return content
>> +
>> +    def print_package_list(self, output=None):
>> +        root_title = "Package Selection for the target"
>> +        root_item = [menu for menu in self.config.get_menus()
>> +            if menu.get_title().lower() == root_title.lower()][0]
>
>
>  This could get better error handling, and the list construct makes it less
> readable than a simple for loop.
Indeed. Done.


Regards,

-- 
Samuel


More information about the buildroot mailing list