Dan McGee wrote: > On Sun, Oct 11, 2009 at 5:12 PM, Allan McRae <[email protected]> wrote: >> Dan McGee wrote: >>> On Thu, Oct 8, 2009 at 9:10 AM, Cedric Staniewski <[email protected]> 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 <[email protected]> >>>> --- >>>> >>> 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?
