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

[Dan McGee]

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