Re: [gentoo-dev] [PATCH] tree-sitter-grammar.eclass: Init

[Michał Górny]

On Sun, 2021-10-17 at 11:42 +0100, Matthew Smith wrote:
> Hi,
>
> Please review tree-sitter-grammar.eclass - a new eclass for building
> tree-sitter grammars provided by the tree-sitter developers.
>
> The grammars are shipped as one or two source files and with no build
> system.  Upstream uses Gyp to build the grammars as a NodeJS module,
> but they are useful standalone to other applications such as text
> editors.
>
> Some new packages utilising the eclass can be viewed on GitHub:
> https://github.com/gentoo/gentoo/pull/22611
>
> Thanks,
> Matthew
>
> ---
>   eclass/tree-sitter-grammar.eclass | 94 +++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)
>   create mode 100644 eclass/tree-sitter-grammar.eclass
>
> diff --git a/eclass/tree-sitter-grammar.eclass
> b/eclass/tree-sitter-grammar.eclass
> new file mode 100644
> index 00000000000..a107b0b1908
> --- /dev/null
> +++ b/eclass/tree-sitter-grammar.eclass
> @@ -0,0 +1,94 @@
> +# Copyright 1999-2021 Gentoo Authors
> +# Distributed under the terms of the GNU General Public License v2
> +
> +# @ECLASS: tree-sitter-grammar.eclass
> +# @MAINTAINER:
> +# Matthew Smith <matt@×××××××××.uk>
> +# Nick Sarnie <sarnex@g.o>
> +# @AUTHOR:
> +# Matthew Smith <matt@×××××××××.uk>
> +# @SUPPORTED_EAPIS: 8
> +# @BLURB: Common functions and variables for Tree Sitter grammars
> +
> +if [[ -z ${_TREE_SITTER_GRAMMAR_ECLASS} ]]; then
> +_TREE_SITTER_GRAMMAR_ECLASS=1
> +
> +case ${EAPI} in
> +       8) ;;
> +       *) die "EAPI=${EAPI:-0} is not supported" ;;
> +esac
> +
> +inherit toolchain-funcs
> +
> +SRC_URI="https://github.com/tree-sitter/${PN}/archive/${TS_PV:-v${PV}}.tar.gz
> -> ${P}.tar.gz"

TS_PV is not documented.  Also, this line asks for wrapping.

> +S="${WORKDIR}"/${PN}-${TS_PV:-${PV}}/src
> +
> +# Needed for tree_sitter/parser.h
> +DEPEND="dev-libs/tree-sitter"
> +
> +EXPORT_FUNCTIONS src_compile src_install
> +
> +# @FUNCTION: get_tsg_abi_ver
> +# @DESCRIPTION:
> +# This internal function determines the ABI version of a grammar
> library based

Then mark it with @INTERNAL?  Also prefixing it with '_' would be a good
idea.

> +# on the package version.
> +get_tsg_abi_ver() {
> +       if ver_test -gt 0.21; then
> +               die "Grammar too new; unknown ABI version"
> +       elif ver_test -ge 0.19.0; then
> +               echo 13
> +       else
> +               die "Grammar too old; unknown ABI version"
> +       fi
> +}
> +
> +# @FUNCTION: tree-sitter-grammar_src_compile
> +# @DESCRIPTION:
> +# Compiles the Tree Sitter parser as a shared library.
> +tree-sitter-grammar_src_compile() {
> +       debug-print-function $FUNCNAME $*

Please keep the prologue consistent with our coding style, i.e.:

  debug-print-function ${FUNCNAME} "${@}"

> +
> +       # Grammars always contain parser.c, and sometimes a scanner.c,
> +       # or scanner.cc.
> +
> +       $(tc-getCC) ${CFLAGS} \
> +                               ${CPPFLAGS} \
> +                               -fPIC \

How about exporting CC etc., and using emake to build them?

i.e. something like

  tc-export CC
  export CFLAGS="${CFLAGS} -fPIC"
  emake parser.o

(also I think it would be less confusing to use the same directory for
output file)

> +                               -c "${S}"/parser.c \
> +                               -o "${WORKDIR}"/parser.o || die
> +
> +       local link=$(tc-getCC)
> +
> +       if [[ -f "${S}/scanner.c" ]]; then
> +               $(tc-getCC) ${CFLAGS} \
> +                                       ${CPPFLAGS} \
> +                                       -fPIC \
> +                                       -c "${S}"/scanner.c \
> +                                       -o "${WORKDIR}"/scanner.o || die
> +       elif [[ -f "${S}/scanner.cc" ]]; then
> +               $(tc-getCXX) ${CXXFLAGS} \
> +                                        ${CPPFLAGS} \
> +                                        -fPIC \
> +                                        -c "${S}"/scanner.cc \
> +                                        -o "${WORKDIR}"/scanner.o || die
> +               link=$(tc-getCXX)
> +       fi
> +
> +       local soname=lib${PN}.so.$(get_tsg_abi_ver)

We've got some helpers like get_libname to make this a little bit more
portable, or rather to introduce less obstacles when people want to fix
portability issues.

> +       ${link} ${LDFLAGS} \
> +                       -shared \
> +                       "${WORKDIR}"/*.o \
> +                       -Wl,-soname ${soname} \
> +                       -o "${WORKDIR}"/${soname} || die
> +}
> +
> +# @FUNCTION: tree-sitter-grammar_src_install
> +# @DESCRIPTION:
> +# Installs the Tree Sitter parser library.
> +tree-sitter-grammar_src_install() {
> +       debug-print-function $FUNCNAME $*
> +
> +       dolib.so "${WORKDIR}"/lib${PN}.so.$(get_tsg_abi_ver)
> +       dosym lib${PN}.so.$(get_tsg_abi_ver) /usr/$(get_libdir)/lib${PN}.so
> +}
> +fi
> --
> 2.33.0
>

-- 
Best regards,
Michał Górny