[pacman-dev] [PATCH] Improve changelog handling through addition of open/read/close functions

[Xavier]

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.