On Sun, Oct 11, 2009 at 5:12 PM, Allan McRae <allan@archlinux.org> wrote:
Dan McGee wrote:
On Thu, Oct 8, 2009 at 9:10 AM, Cedric Staniewski <cedric@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@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