9 Dec
2007
9 Dec
'07
1:11 p.m.
On Sat, Dec 08, 2007 at 11:43:41PM -0600, Dan McGee wrote:
> Thanks to Allan for inspiring all this work on what was one little TODO item
> in the codebase. :)
>
> Change changelog handling so we can now dump a changelog from both installed
> packages and package files (fixes FS#7371). We do this by moving all of the
> machinery to the backend where it should have been in the first place.
>
> The changelog reading is now done through a open/read/close interface
> similar to the fopen/fread/fclose functions (can you guess how it is done?).
> It is buffered by the frontend, so programs using the library can read as
> much or as little as they want at a time.
>
That's an interesting way, it gives more control to the frontend, and avoids
this memory problem.
> @@ -232,28 +234,30 @@ void dump_pkg_files(pmpkg_t *pkg)
> fflush(stdout);
> }
>
> -/* Display the changelog of an installed package
> +/* Display the changelog of a package
> */
> -void dump_pkg_changelog(char *clfile, const char *pkgname)
> +void dump_pkg_changelog(pmpkg_t *pkg)
> {
> - FILE* fp = NULL;
> - char line[PATH_MAX+1];
> + void *fp = NULL;
>
> - if((fp = fopen(clfile, "r")) == NULL)
> - {
> - fprintf(stderr, _("error: no changelog available for '%s'.\n"), pkgname);
> + if((fp = alpm_pkg_changelog_open(pkg)) == NULL) {
> + /* TODO after string freeze use pm_fprintf */
> + fprintf(stderr, _("error: no changelog available for '%s'.\n"),
> + alpm_pkg_get_name(pkg));
> return;
> - }
> - else
> - {
> - while(!feof(fp))
> - {
> - fgets(line, (int)PATH_MAX, fp);
> - printf("%s", line);
> - line[0] = '\0';
> + } else {
> + /* allocate a buffer to get the changelog back in chunks */
> + char buf[CLBUF_SIZE];
> + int ret = 0;
> + while((ret = alpm_pkg_changelog_read(buf, CLBUF_SIZE, pkg, fp))) {
> + if(ret < CLBUF_SIZE) {
> + /* if we hit the end of the file, we need to add a null terminator */
> + *(buf + ret) = '\0';
> + }
> + printf("%s", buf);
> }
> - fclose(fp);
> - return;
> + alpm_pkg_changelog_close(pkg, fp);
> + printf("\n");
> }
> }
>
I see you added a \n there, this wasn't done previously.
I had a look at the changelogs available in my local db, to see how they
ended :
> find /var/lib/pacman/local -name changelog | xargs tail -n1
==> /var/lib/pacman/local/openoffice-base-2.3.1-1/changelog <==
- added ChangeLog
==> /var/lib/pacman/local/rtorrent-0.7.9-1/changelog <==
* Print to the log when close_on_diskspace gets triggered.
==> /var/lib/pacman/local/cmus-2.2.0-3/changelog <==
Added flac and libmad
==> /var/lib/pacman/local/docbook-xml-4.5-1/changelog <==
==> /var/lib/pacman/local/powertop-1.9-1/changelog <==
==> /var/lib/pacman/local/fdm-1.4-1/changelog <==
==> /var/lib/pacman/local/glibc-2.7-7/changelog <==
- added ChangeLog%
So openoffice-base and glibc don't even end with a newline.
cmus and rtorrent end with just one (though that rtorrent package is a custom
one).
docbook-xml, powertop and fdm already have two newlines (so one
empty line).
So in the case of openoffice-base and glibc, adding an extra newline at the
end is necessary. But in the case of docbook-xml, powertop and fdm. it'll
result in several empty lines which isn't very nice either.
Maybe instead of adding an extra newline, we could maybe put a simple
guideline for changelog somewhere, stating that they should end with just one
newline. Then maybe makepkg could correct these automatically. Or it could
be added as a simple check to namcap, if that makes sense and is possible.
Anyway, this is a lot of noise about a really minor and unimportant issue, so
I apologize if you think this is not worth worrying about.