Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   ArchLinux Pacman Development (http://www.linux-archive.org/archlinux-pacman-development/)
-   -   get_filename : use the FILENAME field from the db. (http://www.linux-archive.org/archlinux-pacman-development/55070-get_filename-use-filename-field-db.html)

"Dan McGee" 02-13-2008 11:35 PM

get_filename : use the FILENAME field from the db.
 
On Feb 13, 2008 5:06 PM, Chantry Xavier <shiningxc@gmail.com> wrote:
> This fixes FS#9547.
>
> The get_filename function tried to construct the filename from name, version
> and arch, instead of directly using the filename field from the database,
> and got it wrong in most cases (with both core/extra and community repo).
>
> Since this field was introduced in 3.0, it should be safe to use it now in
> 3.1.
>
> Added a quick fix to pactest to also generate the FILENAME field in its sync
> databases.
>
> Signed-off-by: Chantry Xavier <shiningxc@gmail.com>
> ---
> lib/libalpm/package.c | 20 +++++++-------------
> lib/libalpm/sync.c | 11 ++++++++++-
> pactest/pmdb.py | 2 ++
> 3 files changed, 19 insertions(+), 14 deletions(-)
>
>
> diff --git a/lib/libalpm/package.c b/lib/libalpm/package.c
> index 937ee3e..d3351a4 100644
> --- a/lib/libalpm/package.c
> +++ b/lib/libalpm/package.c
> @@ -107,6 +107,7 @@ int SYMEXPORT alpm_pkg_checkmd5sum(pmpkg_t *pkg)
> char *fpath;
> char *md5sum = NULL;
> int retval = 0;
> + const char *filename;
Super small issue, but want to put all the char declarations are on
consecutive lines, followed by the int one? Keeps the logically
grouped a bit better.

> ALPM_LOG_FUNC;
>
> @@ -115,7 +116,10 @@ int SYMEXPORT alpm_pkg_checkmd5sum(pmpkg_t *pkg)
> ASSERT(pkg->origin == PKG_FROM_CACHE, RET_ERR(PM_ERR_PKG_INVALID, -1));
> ASSERT(pkg->origin_data.db != handle->db_local, RET_ERR(PM_ERR_PKG_INVALID, -1));
>
> - fpath = _alpm_filecache_find(alpm_pkg_get_filename(pkg));
> + filename = alpm_pkg_get_filename(pkg);
> + ASSERT(filename[0] != '', RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
> +
> + fpath = _alpm_filecache_find(filename);
Good.

> md5sum = alpm_get_md5sum(fpath);
>
> if(md5sum == NULL) {
> @@ -162,18 +166,8 @@ const char SYMEXPORT *alpm_pkg_get_filename(pmpkg_t *pkg)
> ASSERT(handle != NULL, return(NULL));
> ASSERT(pkg != NULL, return(NULL));
>
> - if(!strlen(pkg->filename)) {
> - /* construct the file name, it's not in the desc file */
> - if(pkg->origin == PKG_FROM_CACHE && !(pkg->infolevel & INFRQ_DESC)) {
> - _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC);
> - }
> - if(pkg->arch && strlen(pkg->arch) > 0) {
> - snprintf(pkg->filename, PKG_FILENAME_LEN, "%s-%s-%s" PKGEXT,
> - pkg->name, pkg->version, pkg->arch);
> - } else {
> - snprintf(pkg->filename, PKG_FILENAME_LEN, "%s-%s" PKGEXT,
> - pkg->name, pkg->version);
> - }
> + if(pkg->origin == PKG_FROM_CACHE && !(pkg->infolevel & INFRQ_DESC)) {
> + _alpm_db_read(pkg->origin_data.db, pkg, INFRQ_DESC);
> }
Looks great.


> return pkg->filename;
> diff --git a/lib/libalpm/sync.c b/lib/libalpm/sync.c
> index ced20c5..e03a757 100644
> --- a/lib/libalpm/sync.c
> +++ b/lib/libalpm/sync.c
> @@ -689,6 +689,7 @@ static alpm_list_t *pkg_upgrade_delta_path(pmpkg_t *newpkg, pmdb_t *db_local)
>
> if(oldpkg) {
> const char *oldname = alpm_pkg_get_filename(oldpkg);
> + ASSERT(oldname[0] != '', RET_ERR(PM_ERR_PKG_INVALID_NAME, NULL));
Hmm. I wonder if we need some more testing here. Did this never return
the empty string in any pactests, or anything else? I also don't know
that a locally installed package will store what filename it was
installed from. I wonder if we just broke all of the delta
assumptions. Or what is 'oldpkg' here?

> char *oldpath = _alpm_filecache_find(oldname);
>
> if(oldpath) {
> @@ -726,7 +727,9 @@ static alpm_list_t *pkg_upgrade_delta_path(pmpkg_t *newpkg, pmdb_t *db_local)
> */
> unsigned long SYMEXPORT alpm_pkg_download_size(pmpkg_t *newpkg, pmdb_t *db_local)
> {
> - char *fpath = _alpm_filecache_find(alpm_pkg_get_filename(newpkg) );
> + const char *fname = alpm_pkg_get_filename(newpkg);
> + ASSERT(fname[0] != '', RET_ERR(PM_ERR_PKG_INVALID_NAME, 0));
> + char *fpath = _alpm_filecache_find(fname);
> unsigned long size = 0;
Good.

>
> if(fpath) {
> @@ -950,6 +953,8 @@ static int test_pkg_md5sum(pmtrans_t *trans, pmpkg_t *pkg, alpm_list_t **data)
> int ret = 0;
>
> filename = alpm_pkg_get_filename(pkg);
> + ASSERT(filename[0] != '', RET_ERR(PM_ERR_PKG_INVALID_NAME, 1));
> +
> md5sum = alpm_pkg_get_md5sum(pkg);
>
> ret = test_md5sum(trans, filename, md5sum, data);
> @@ -995,6 +1000,7 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
> const char *fname = NULL;
>
> fname = alpm_pkg_get_filename(spkg);
> + ASSERT(fname[0] != '', RET_ERR(PM_ERR_PKG_INVALID_NAME, -1));
> if(trans->flags & PM_TRANS_FLAG_PRINTURIS) {
> EVENT(trans, PM_TRANS_EVT_PRINTURI, (char *)alpm_db_get_url(current),
> (char *)fname);
> @@ -1187,6 +1193,9 @@ int _alpm_sync_commit(pmtrans_t *trans, pmdb_t *db_local, alpm_list_t **data)
> char *fpath;
>
> fname = alpm_pkg_get_filename(spkg);
> + if(fname[0] == '') {
> + goto error;
> + }
> /* Loop through the cache dirs until we find a matching file */
> fpath = _alpm_filecache_find(fname);
>
> diff --git a/pactest/pmdb.py b/pactest/pmdb.py
> index af39200..d0a4f6d 100755
> --- a/pactest/pmdb.py
> +++ b/pactest/pmdb.py
> @@ -261,6 +261,8 @@ class pmdb:
> if pkg.reason:
> data.append(_mksection("REASON", pkg.reason))
> else:
> + if pkg.filename():
> + data.append(_mksection("FILENAME", pkg.filename()))
> if pkg.replaces:
> data.append(_mksection("REPLACES", pkg.replaces))
> if pkg.force:
>
> --
> 1.5.4

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

"Dan McGee" 02-15-2008 06:33 PM

get_filename : use the FILENAME field from the db.
 
On Fri, Feb 15, 2008 at 1:08 PM, Nathan Jones <nathanj@insightbb.com> wrote:
> On Fri, Feb 15, 2008 at 05:37:39PM +0100, Xavier wrote:
> > Btw, if you are interested in doing that, it would be much appreciated :)
> > Since it's your code after all, I think you would be more efficient.
> > But there is no hurry anyway since deltas aren't in use yet.
>
> Yes I was planning on doing it. I should have mentioned that in my
> email.
>
> This last week I've been working on fixing the gzip/md5sum problem with
> deltas in order to add delta creation to repo-add. I think I will post
> that code tonight and begin on this soon.

If you want any help or anything, feel free to ping me on IRC or
jabber and we can work some details out or I can give you some
immediate feedback on anything you have questions on.

-Dan

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev

"Dan McGee" 02-15-2008 10:29 PM

get_filename : use the FILENAME field from the db.
 
On Fri, Feb 15, 2008 at 1:33 PM, Dan McGee <dpmcgee@gmail.com> wrote:
> On Fri, Feb 15, 2008 at 1:08 PM, Nathan Jones <nathanj@insightbb.com> wrote:
> > On Fri, Feb 15, 2008 at 05:37:39PM +0100, Xavier wrote:
> > > Btw, if you are interested in doing that, it would be much appreciated :)
> > > Since it's your code after all, I think you would be more efficient.
> > > But there is no hurry anyway since deltas aren't in use yet.
> >
> > Yes I was planning on doing it. I should have mentioned that in my
> > email.
> >
> > This last week I've been working on fixing the gzip/md5sum problem with
> > deltas in order to add delta creation to repo-add. I think I will post
> > that code tonight and begin on this soon.
>
> If you want any help or anything, feel free to ping me on IRC or
> jabber and we can work some details out or I can give you some
> immediate feedback on anything you have questions on.

So how about a delta file in the sync dbs like the following:

%DELTAS%
oldfile.pkg.tar.gz 000md5sum000 newfile.pkg.tar.gz 000md5sum000
deltafile.delta 000md5sum000 23423
# or in bash format
$oldfile $oldmd5 $newfile $newmd5 $deltafile $deltamd5 $deltasize

I think you have an existing algorithm in place that we can adapt to
this. I'll psuedocode it:

read in delta lines
build a tree of possible delta paths from $syncfilename back to each
$oldfile until we can't go further, or until we find an $oldfile that
we can lstat and the md5sum matches.
find shortest path of our possible paths
use this delta path (and since we have md5sums to verify the whole
way, we can be more sure before we start downloading that it will
work)

Note that this whole process never worries about parsing the filename
into parts, or for that matter, even opening the package file and
looking at PKGINFO.

-Dan

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev


All times are GMT. The time now is 04:25 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.