Xavier wrote:

diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 172456d..3d7e74d 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -491,6 +491,86 @@ alpm_list_t SYMEXPORT *alpm_pkg_get_backup(pmpkg_t *pkg) return pkg->backup; }

+char SYMEXPORT *alpm_pkg_get_changelog(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + int ret = ARCHIVE_OK; + char clfile[PATH_MAX]; + FILE *fp = NULL; + struct archive *archive = NULL; + struct archive_entry *entry; + char *changelog = NULL, *p = NULL; + int bufsize = 512, size = 0, n = 0; + int done = 0, found = 0; + + if(pkg->origin == PKG_FROM_CACHE) { + snprintf(clfile, PATH_MAX, "%s/%s/%s-%s/changelog", + alpm_option_get_dbpath(), + alpm_db_get_name(handle->db_local), + alpm_pkg_get_name(pkg), + alpm_pkg_get_version(pkg)); + fp = fopen(clfile, "r"); + if(fp != NULL) { + found = 1; + } + } else if(pkg->origin == PKG_FROM_FILE) { + const char *pkgfile = pkg->origin_data.file; + + if((archive = archive_read_new()) == NULL) { + RET_ERR(PM_ERR_LIBARCHIVE_ERROR, NULL); + } + + archive_read_support_compression_all(archive); + archive_read_support_format_all(archive); + + if (archive_read_open_filename(archive, pkgfile, + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + RET_ERR(PM_ERR_PKG_OPEN, NULL); + } + + while((ret = archive_read_next_header(archive, &entry)) == ARCHIVE_OK) { + const char *entry_name = archive_entry_pathname(entry); + + if(strcmp(entry_name, ".CHANGELOG") == 0) { + found = 1; + break; + } + } + } + + if(!found) { + return(NULL);

You're missing an archive_read_finish(archive) before the return. Andrew

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. Unfortunately, I could not implement a changelog_feof function due to some shortcomings of libarchive. However, I left the stub code in there, commented out, in case it becomes possible later or anyone wants to take a stab at it. Original-work-by: Allan McRae Improved-by: Chantry Xavier Signed-off-by: Dan McGee --- lib/libalpm/alpm.h | 5 ++ lib/libalpm/package.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/package.c | 38 +++++++++-------- src/pacman/package.h | 2 +- src/pacman/query.c | 9 +---- 5 files changed, 137 insertions(+), 26 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 1e18ad9..c98adc5 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -216,6 +216,11 @@ alpm_list_t *alpm_pkg_get_deltas(pmpkg_t *pkg); alpm_list_t *alpm_pkg_get_replaces(pmpkg_t *pkg); alpm_list_t *alpm_pkg_get_files(pmpkg_t *pkg); alpm_list_t *alpm_pkg_get_backup(pmpkg_t *pkg); +void *alpm_pkg_changelog_open(pmpkg_t *pkg); +size_t alpm_pkg_changelog_read(void *ptr, size_t size, + const pmpkg_t *pkg, const void *fp); +/*int alpm_pkg_changelog_feof(const pmpkg_t *pkg, void *fp);*/ +int alpm_pkg_changelog_close(const pmpkg_t *pkg, void *fp); unsigned short alpm_pkg_has_scriptlet(pmpkg_t *pkg); unsigned long alpm_pkg_download_size(pmpkg_t *newpkg, pmdb_t *db_local); diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c index 172456d..efb54b0 100644 --- a/lib/libalpm/package.c +++ b/lib/libalpm/package.c @@ -491,6 +491,115 @@ alpm_list_t SYMEXPORT *alpm_pkg_get_backup(pmpkg_t *pkg) return pkg->backup; } +/** + * Open a package changelog for reading. Similar to fopen in functionality, + * except that the returned 'file stream' could really be from an archive + * as well as from the database. + * @param pkg the package to read the changelog of (either file or db) + * @return a 'file stream' to the package changelog + */ +void SYMEXPORT *alpm_pkg_changelog_open(pmpkg_t *pkg) +{ + ALPM_LOG_FUNC; + + /* Sanity checks */ + ASSERT(handle != NULL, return(NULL)); + ASSERT(pkg != NULL, return(NULL)); + + if(pkg->origin == PKG_FROM_CACHE) { + char clfile[PATH_MAX]; + snprintf(clfile, PATH_MAX, "%s/%s/%s-%s/changelog", + alpm_option_get_dbpath(), + alpm_db_get_name(handle->db_local), + alpm_pkg_get_name(pkg), + alpm_pkg_get_version(pkg)); + return fopen(clfile, "r"); + } else if(pkg->origin == PKG_FROM_FILE) { + struct archive *archive = NULL; + struct archive_entry *entry; + const char *pkgfile = pkg->origin_data.file; + int ret = ARCHIVE_OK; + + if((archive = archive_read_new()) == NULL) { + RET_ERR(PM_ERR_LIBARCHIVE_ERROR, NULL); + } + + archive_read_support_compression_all(archive); + archive_read_support_format_all(archive); + + if (archive_read_open_filename(archive, pkgfile, + ARCHIVE_DEFAULT_BYTES_PER_BLOCK) != ARCHIVE_OK) { + RET_ERR(PM_ERR_PKG_OPEN, NULL); + } + + while((ret = archive_read_next_header(archive, &entry)) == ARCHIVE_OK) { + const char *entry_name = archive_entry_pathname(entry); + + if(strcmp(entry_name, ".CHANGELOG") == 0) { + return(archive); + } + } + /* we didn't find a changelog */ + archive_read_finish(archive); + errno = ENOENT; + } + return(NULL); +} + +/** + * Read data from an open changelog 'file stream'. Similar to fread in + * functionality, this function takes a buffer and amount of data to read. + * @param ptr a buffer to fill with raw changelog data + * @param size the size of the buffer + * @param pkg the package that the changelog is being read from + * @param fp a 'file stream' to the package changelog + * @return the number of characters read, or 0 if there is no more data + */ +size_t SYMEXPORT alpm_pkg_changelog_read(void *ptr, size_t size, + const pmpkg_t *pkg, const void *fp) +{ + size_t ret = 0; + if(pkg->origin == PKG_FROM_CACHE) { + ret = fread(ptr, 1, size, (FILE*)fp); + } else if(pkg->origin == PKG_FROM_FILE) { + ret = archive_read_data((struct archive*)fp, ptr, size); + } + return(ret); +} + +/* +int SYMEXPORT alpm_pkg_changelog_feof(const pmpkg_t *pkg, void *fp) +{ + int ret = 0; + if(pkg->origin == PKG_FROM_CACHE) { + ret = feof((FILE*)fp); + } else if(pkg->origin == PKG_FROM_FILE) { + // note: this doesn't quite work, no feof in libarchive + ret = archive_read_data((struct archive*)fp, NULL, 0); + } + return(ret); +} +*/ + +/** + * Close a package changelog for reading. Similar to fclose in functionality, + * except that the 'file stream' could really be from an archive as well as + * from the database. + * @param pkg the package that the changelog was read from + * @param fp a 'file stream' to the package changelog + * @return whether closing the package changelog stream was successful + */ +int SYMEXPORT alpm_pkg_changelog_close(const pmpkg_t *pkg, void *fp) +{ + int ret = 0; + if(pkg->origin == PKG_FROM_CACHE) { + ret = fclose((FILE*)fp); + } else if(pkg->origin == PKG_FROM_FILE) { + ret = archive_read_finish((struct archive *)fp); + } + return(ret); +} + unsigned short SYMEXPORT alpm_pkg_has_scriptlet(pmpkg_t *pkg) { ALPM_LOG_FUNC; diff --git a/src/pacman/package.c b/src/pacman/package.c index ac3f820..ba57214 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -34,6 +34,8 @@ #include "package.h" #include "util.h" +#define CLBUF_SIZE 4096 + /* Display the content of a package * * level: <0 - sync package [-Si] @@ -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"); } } diff --git a/src/pacman/package.h b/src/pacman/package.h index 0e4bb0f..7dfc054 100644 --- a/src/pacman/package.h +++ b/src/pacman/package.h @@ -28,7 +28,7 @@ void dump_pkg_sync(pmpkg_t *pkg, const char *treename); void dump_pkg_backups(pmpkg_t *pkg); void dump_pkg_files(pmpkg_t *pkg); -void dump_pkg_changelog(char *clfile, const char *pkgname); +void dump_pkg_changelog(pmpkg_t *pkg); #endif /* _PM_PACKAGE_H */ diff --git a/src/pacman/query.c b/src/pacman/query.c index 8a8765b..1307077 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -312,14 +312,7 @@ static void display(pmpkg_t *pkg) dump_pkg_files(pkg); } if(config->op_q_changelog) { - char changelog[PATH_MAX]; - /* TODO should be done in the backend- no raw DB stuff up front */ - snprintf(changelog, PATH_MAX, "%s/%s/%s-%s/changelog", - alpm_option_get_dbpath(), - alpm_db_get_name(db_local), - alpm_pkg_get_name(pkg), - alpm_pkg_get_version(pkg)); - dump_pkg_changelog(changelog, alpm_pkg_get_name(pkg)); + dump_pkg_changelog(pkg); } if(!config->op_q_info && !config->op_q_list && !config->op_q_changelog) { if (!config->quiet) { -- 1.5.3.7

On Dec 9, 2007 7:11 AM, Xavier  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