[pacman-dev] [PATCH] Introduce new PKGBUILD variable `changelog`

[Dan McGee]

On Thu, Oct 8, 2009 at 9:10 AM, Cedric Staniewski <cedric at gmx.ca> wrote:
> Currently, a changelog is added to a package if a specific file with a
> hardcoded name exists in the PKGBUILD's directory. This approach is not
> pretty and also inconsistent with the handling of install files, but it
> works.
>
> With the introduction of split PKGBUILDs, however, a drawback in this
> old behavior has arisen: you only have the possibility to include one
> specific changelog file in either every package defined in the PKGBUILD
> or in none.
>
> The use of an additional variable, `changelog`, works around this issue
> and makes it possible to include a changelog in only some of the
> packages, and besides, each package of the PKGBUILD can have its own
> changelog file.
>
> Signed-off-by: Cedric Staniewski <cedric at gmx.ca>
> ---
Before we pull this, can you (or Allan) make some changes? It isn't
quite ready for primetime yet.

>  PKGBUILD-split.proto  |    2 ++
>  PKGBUILD.proto        |    1 +
>  contrib/PKGBUILD.vim  |    9 +++++++++
>  doc/PKGBUILD.5.txt    |    8 +++++++-
>  scripts/makepkg.sh.in |   23 ++++++++++++++++-------
>  5 files changed, 35 insertions(+), 8 deletions(-)
>
> diff --git a/PKGBUILD-split.proto b/PKGBUILD-split.proto
> index 08f98d6..11ceff2 100644
> --- a/PKGBUILD-split.proto
> +++ b/PKGBUILD-split.proto
> @@ -21,6 +21,7 @@ replaces=()
>  backup=()
>  options=()
>  install=
> +changelog=
>  source=($pkgbase-$pkgver.tar.gz)
>  noextract=()
>  md5sums=() #generate with 'makepkg -g'
> @@ -44,6 +45,7 @@ package_pkg1() {
>   backup=()
>   options=()
>   install=
> +  changelog=
>
>   cd "$srcdir/$pkgbase-$pkgver"
>   make DESTDIR="$pkgdir/" install-pkg1
> diff --git a/PKGBUILD.proto b/PKGBUILD.proto
> index eddcf75..93da0fc 100644
> --- a/PKGBUILD.proto
> +++ b/PKGBUILD.proto
> @@ -21,6 +21,7 @@ replaces=()
>  backup=()
>  options=()
>  install=
> +changelog=
>  source=($pkgname-$pkgver.tar.gz)
>  noextract=()
>  md5sums=() #generate with 'makepkg -g'
> diff --git a/contrib/PKGBUILD.vim b/contrib/PKGBUILD.vim
> index 994eacc..af0f981 100644
> --- a/contrib/PKGBUILD.vim
> +++ b/contrib/PKGBUILD.vim
> @@ -115,6 +115,12 @@ syn match pbValidInstall /\([[:alnum:]]\|\$\|+\|-\|_\)*\.install/ contained
>  syn match pbIllegalInstall /[^=]/ contained contains=pbValidInstall
>  syn match pbInstallGroup /^install=.*/ contains=pb_k_install,pbValidInstall,pbIllegalInstall,shDeref,shDoubleQuote,shSingleQuote
>
> +" changelog
> +syn keyword pb_k_changelog changelog contained
> +syn match pbValidChangelog /\([[:alnum:]]\|\$\|+\|-\|_\)*/ contained
> +syn match pbIllegalChangelog /[^=]/ contained contains=pbValidChangelog
> +syn match pbChangelogGroup /^changelog=.*/ contains=pb_k_changelog,pbValidChangelog,pbIllegalChangelog,shDeref,shDoubleQuote,shSingleQuote
> +
>  " source:
>  " XXX remove source from shStatement, fix strange bug
>  syn clear shStatement
> @@ -212,6 +218,9 @@ hi def link pb_k_provides pbKeywords
>  hi def link pbIllegalInstall Error
>  hi def link pb_k_install pbKeywords
>
> +hi def link pbIllegalChangelog Error
> +hi def link pb_k_changelog pbKeywords
> +
>  hi def link pb_k_source pbKeywords
>  hi def link pbIllegalSource Error
>
> diff --git a/doc/PKGBUILD.5.txt b/doc/PKGBUILD.5.txt
> index e6f6edf..435a148 100644
> --- a/doc/PKGBUILD.5.txt
> +++ b/doc/PKGBUILD.5.txt
> @@ -79,6 +79,12 @@ similar to `$_basekernver`.
>        be copied into the package by makepkg. It does not need to be included
>        in the source array (e.g. `install=pkgname.install`).
>
> +*changelog*::
> +       Specifies a changelog file which is supposed to be included in the package.
"supposed"? Since we fail hard if it is missing, just keep the
language more direct.
"Specifies a changelog file which is to be included in the package."
In reality, we should keep the language for this and the install file
as similar as possible, as I believe they are under the same set of
rules.

> +       It is required that this file is placed into the same directory as the
> +       PKGBUILD, and makepkg will copy it into the package. The file must not be
> +       included in the source array (e.g. `changelog=$pkgname.changelog`).
> +
>  *source (array)*::
>        An array of source files required to build the package. Source files
>        must either reside in the same directory as the PKGBUILD file, or be a
> @@ -271,7 +277,7 @@ All options and directives for the split packages default to the global values g
>  within the PKGBUILD. However, some of these can be overridden within each split
>  package's packaging function. The following variables can be overridden: `pkgdesc`,
>  `license`, `groups`, `depends`, `optdepends`, `provides`, `conflicts`, `replaces`,
> -`backup`, `options` and `install`.
> +`backup`, `options`, `install` and `changelog`.
>
>  An optional global directive is available when building a split package:
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index 2941a5d..88487da 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -47,7 +47,7 @@ pkgdir="$startdir/pkg"
>  packaging_options=('strip' 'docs' 'libtool' 'emptydirs' 'zipman' 'purge')
>  other_options=('ccache' 'distcc' 'makeflags' 'force')
>  splitpkg_overrides=('pkgdesc' 'license' 'groups' 'depends' 'optdepends' 'provides' \
> -                    'conflicts' 'replaces' 'backup' 'options' 'install')
> +                    'conflicts' 'replaces' 'backup' 'options' 'install' 'changelog')
>  readonly -a packaging_options other_options splitpkg_overrides
>
>  # Options
> @@ -991,9 +991,9 @@ create_package() {
>        fi
>
>        # do we have a changelog?
> -       if [ -f "$startdir/ChangeLog" ]; then
> +       if [ -n "$changelog" ]; then
>                msg2 "$(gettext "Adding package changelog...")"
> -               cp "$startdir/ChangeLog" .CHANGELOG
> +               cp "$startdir/$changelog" .CHANGELOG
>                comp_files="$comp_files .CHANGELOG"
>        fi
>
> @@ -1059,9 +1059,13 @@ create_srcpackage() {
>                fi
>        fi
>
> -       if [ -f ChangeLog ]; then
> -               msg2 "$(gettext "Adding %s...")" "ChangeLog"
> -               ln -s "${startdir}/ChangeLog" "${srclinks}/${pkgbase}"
> +       if [ -n "$changelog" ]; then
> +               if [ -f "$changelog" ]; then
> +                       msg2 "$(gettext "Adding package changelog...")"
> +                       ln -s "${startdir}/$changelog" "${srclinks}/${pkgbase}/"
> +               else
> +                       error "$(gettext "Changelog file %s not found.")" "$changelog"
> +               fi
>        fi
>
>        local netfile
> @@ -1193,6 +1197,11 @@ check_sanity() {
>                return 1
>        fi
>
> +       if [ -n "$changelog" -a ! -f "$changelog" ]; then
> +               error "$(gettext "Changelog file (%s) does not exist.")" "$changelog"
Not cool for translators to have two messages that say the same thing,
but are not the same. Please choose one (that hopefully resembles the
install script missing message).
However, why do we do this check in two places? Is it due to split
packages? And we fail in one, but not in the other.

> +               return 1
> +       fi
> +
>        local valid_options=1
>        local opt known kopt
>        for opt in ${options[@]}; do
> @@ -1646,7 +1655,7 @@ if [ "$ASROOT" -eq 0 \
>  fi
>
>  unset pkgname pkgbase pkgver pkgrel pkgdesc url license groups provides
> -unset md5sums replaces depends conflicts backup source install build
> +unset md5sums replaces depends conflicts backup source install changelog build
>  unset makedepends optdepends options noextract
>
>  BUILDFILE=${BUILDFILE:-$BUILDSCRIPT}
> --

Thanks for doing a very good sweep with this patch and making all the
necessary doc and other script updates, it is appreciated.

-Dan