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

[Dan McGee]

On Dec 9, 2007 7:11 AM, Xavier <shiningxc at gmail.com> wrote:
> 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.

Want to make a ChangeLog.proto to stick in contrib/ ? You could make a
few sample changelog entries and make one of them state that the file
should end with one blank line.

I tested with the glibc package (both package and installed in my
local db) thus the reason I added the newline. :)

-Dan