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
