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

[Cedric Staniewski]

Dan McGee wrote:
> On Sun, Oct 11, 2009 at 5:12 PM, Allan McRae <allan at archlinux.org> wrote:
>> Dan McGee wrote:
>>> 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.
>>>
>>>
>>>> <snip>
>>>> +*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.
>>>
>>>
>> Fixed.
>>
>>>> @@ -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.
>>>
>> These exactly the same as the two messages for install files.  I can make
>> the two messages the same and just pull in a similar change for the install
>> file.
>>
>> And the second check is because the check_sanity script misses variables in
>> package() functions (http://bugs.archlinux.org/task/16004).  The lack of
>> fail in the second one is a bit weird, but that chack is done at the end of
>> the packaging rather than at the start where we do fail.
> 
> Thanks for addressing the issues! And yeah, that is what I expected,
> although that is a shame.
> 
> -Dan
> 

Thanks for resolving the existing issues, Allan.
So, can I assume none is left for this patch?