FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Debian > Debian dpkg

 
 
LinkBack Thread Tools
 
Old 11-25-2011, 03:21 PM
Raphael Hertzog
 
Default Review of pu/multiarch/master

Hi Guillem,

I took some time to review your work in progress (pu/multiarch/master
while it was pointing to 5602d63c7669a9cb5ba293faff8cf85b3accc746).

I have pushed some small fixes for you in pu/multiarch/for-guillem,
there are "fixup!" commits ready to be "autosquashed" by you. Most
of the changes ary typo/doc updates but there's a bit more, see below.

There's also a new pu/multiarch/full which is just the
pu/multiarch/for-guillem with everything squashed and rebased on latest
master.

Some other comments:

*** commit bbcd140fa85a8aa42ab8f52a86ae91591cdba6f0
“dpkg: Update on-disk database to use a multiarch compliant layout”

| pkg_infodb_init(void)
| {
| char *db_format_file, *db_format_file_new;
|
| db_format_file_new = dpkg_db_get_path(INFODIR "/format" NEWDBEXT);

It's a bit unfortunate that we have to hardcode the filename that is in fact
controlled by the new atomic_file module. I would suggest to enhance atomic_file
to be able to export the names for the various files.

I have add a few commits to squash for this. And also to hide NEWDBEXT/OLDDBEXT
in atomic-file.

*** commit a8c90816abb1f8b4c077e92a04faed5311c7483f
“libdpkg: Change pkg_db_find_pkg() to create new arch instances”

It's tagged "Sponsored-by: Linaro" but you're marked as author,
I think it's a mistake due to the fact that you split a commit of mine.

Cheers,
--
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/go/ulule-rh/


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111125162132.GA8044@rivendell.home.ouaza.com">ht tp://lists.debian.org/20111125162132.GA8044@rivendell.home.ouaza.com
 
Old 11-26-2011, 12:05 AM
Raphael Hertzog
 
Default Review of pu/multiarch/master

On Fri, 25 Nov 2011, Raphael Hertzog wrote:
> Some other comments:

Following up... I have updated the test suite to work with the new
--add-architecture/--remove-architecture. Then I tried to run the test
suite and it doesn't work. I identified several issues:

1/ The decision to allow pkgbin->installed.arch / pkgbin->available.arch
to be NULL causes segfault while checking dependencies on virtual
packages.

I pushed a new commit that modifies pkg_db_find_set() to initialize
pkgset->pkg.installed.arch & pkgset->pkg.installed.available. It still
enables to output different warnings for missing architecture field
and empty architecture field in parsedb() since that code uses first a
statically allocated pkgset and not one allocated by pkg_db_find_set().

commit a8c90816abb1f8b4c077e92a04faed5311c7483f
“libdpkg: Change pkg_db_find_pkg() to create new arch instances”

I adjusted this commit to drop a case in the corresponding assert().

2/ --remove-architecture doesn't work because modstatdb_open() calls
dpkg_arch_load_list() a second time and all the struct dpkg_arch are
recreated.

commit 6f9e1a99f6f048e28959caa3c4d79bf0256c84a6
“libdpkg: Add new dpkg_arch database interface”

I added a static boolean to avoid loading the list twice.

3/ pkgbin->name_arch is not initialized to NULL

commit 2a911ab5c605c8a9cc939d04fa30fd5e646640b7
“Add new package name accessors"

I added the missing initialization.

4/ Triggers are no longer working as expected with M-A: same packages.
process_archive() calls trig_parse_ci() while the interesting content is
only in pkg->available but it doesn't forward this information further
down... trig_file_interests_update() calls pkg_name() and thus stores a
package name without its expected architecture qualifier (since
pkg->installed is empty).

If you install another instance of the package, the unqualified entry
gets his qualifier by the magic of reloading it. But the newly added
entry still doesn't have it. This is then an error because "foo:<native>"
and "foo" are the same package...

commit 6e244111c74f058e13f31140e342e4d6518f0e56
“Update triggers support to understand multi-arch”

I fixed this by extending trig_options and ensuring we can initialize
it right from trig_parse_ci() and let it trickle down up to where we need
it.


With all those fixes, the test-suite passes again. Thus I have update
my test-build branch with this code.

All the fixes can be reviewed individually in pu/multiarch/for-guillem:
http://anonscm.debian.org/gitweb/?p=users/hertzog/dpkg.git;a=log;h=refs/heads/pu/multiarch/for-guillem

Cheers,
--
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/go/ulule-rh/


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111126010540.GA18646@rivendell.home.ouaza.com">h ttp://lists.debian.org/20111126010540.GA18646@rivendell.home.ouaza.com
 
Old 11-26-2011, 01:02 AM
Jonathan Nieder
 
Default Review of pu/multiarch/master

Raphael Hertzog wrote:

> Then I tried to run the test
> suite and it doesn't work. I identified several issues:

Ah. I should have read my email before sending a redundant report.

[...]
> I pushed a new commit that modifies pkg_db_find_set() to initialize
> pkgset->pkg.installed.arch & pkgset->pkg.installed.available.

I don't see this at <git://git.debian.org/~hertzog/dpkg.git>. Did you
remember to push to wagner?

Thanks for your work,
Jonathan


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111126020219.GA17452@elie.hsd1.il.comcast.net">h ttp://lists.debian.org/20111126020219.GA17452@elie.hsd1.il.comcast.net
 
Old 11-26-2011, 01:08 AM
Jonathan Nieder
 
Default Review of pu/multiarch/master

Jonathan Nieder wrote:

> Did you remember to push to wagner?

Found it (on the pu/multiarch/for-guillem branch). Sorry for the
noise.


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111126020829.GA17550@elie.hsd1.il.comcast.net">h ttp://lists.debian.org/20111126020829.GA17550@elie.hsd1.il.comcast.net
 
Old 11-27-2011, 10:22 PM
Guillem Jover
 
Default Review of pu/multiarch/master

On Fri, 2011-11-25 at 17:21:32 +0100, Raphael Hertzog wrote:
> I took some time to review your work in progress (pu/multiarch/master
> while it was pointing to 5602d63c7669a9cb5ba293faff8cf85b3accc746).
>
> I have pushed some small fixes for you in pu/multiarch/for-guillem,
> there are "fixup!" commits ready to be "autosquashed" by you. Most
> of the changes ary typo/doc updates but there's a bit more, see below.

Thanks, I've done a quick scan, merged some, and fixed others in some
other way. I've been since the 23rd on a trip, but will be back
tomorrow, when I'll handle the rest of the issues you found, the
remaning interface changes, and will send a status mail.

> *** commit a8c90816abb1f8b4c077e92a04faed5311c7483f
> “libdpkg: Change pkg_db_find_pkg() to create new arch instances”
>
> It's tagged "Sponsored-by: Linaro" but you're marked as author,
> I think it's a mistake due to the fact that you split a commit of mine.

Indeed, there was another one like that, either due to commit splitting
or due to author being switched on conflict resolution. I've fixed
those, but there might be still other similar issues, in any case I
always check for this kinds of issues when merging into master.

regards,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111127232211.GA16825@gaara.hadrons.org">http://lists.debian.org/20111127232211.GA16825@gaara.hadrons.org
 
Old 12-02-2011, 01:43 PM
Raphael Hertzog
 
Default Review of pu/multiarch/master

Hi,

On Mon, 28 Nov 2011, Guillem Jover wrote:
> Thanks, I've done a quick scan, merged some, and fixed others in some
> other way. I've been since the 23rd on a trip, but will be back
> tomorrow, when I'll handle the rest of the issues you found, the
> remaning interface changes, and will send a status mail.

I saw you merged fixes for almost everything but your fix for the trigger
part is not complete enough. I pushed again a new patch in
pu/multiarch/for-guillem (it's also attached).

The fact that the pkgbin pointer is stored in the filetriggers list is a
bit unsatisfactory as it might become stale when the package is installed
(i.e. it should in theory be updated to point to the installed pkgbin
instead of the available one). Though in practice the cases where the
pkgbin for available will be updated to something else in the same dpkg
run are close to zero.

Cheers,
--
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/
commit 75a5fdf21da8b911c19896a4153441688eae5180
Author: Raphaël Hertzog <hertzog@debian.org>
Date: Fri Dec 2 12:41:13 2011 +0100

fixup! Update triggers support to understand multi-arch

Passing pkgbin is not enough, one must also make use of it...

diff --git a/lib/dpkg/triglib.c b/lib/dpkg/triglib.c
index 0b5da83..c57d03b 100644
--- a/lib/dpkg/triglib.c
+++ b/lib/dpkg/triglib.c
@@ -339,7 +339,7 @@ trk_explicit_interest_change(const char *trig, struct pkginfo *pkg,
if (slash)
*slash = '';
pkg_spec_parse(&pkgspec, name);
- match = pkg_spec_match_pkg(&pkgspec, pkg, &pkg->installed);
+ match = pkg_spec_match_pkg(&pkgspec, pkg, pkgbin);
pkg_spec_destroy(&pkgspec);
free(name);

@@ -350,7 +350,7 @@ trk_explicit_interest_change(const char *trig, struct pkginfo *pkg,

}
if (signum > 0) {
- fprintf(file->fp, "%s%s
", pkg_name(pkg, pno_nonambig),
+ fprintf(file->fp, "%s%s
", pkgbin_name(pkg, pkgbin, pno_nonambig),
(opts == trig_noawait) ? "/noawait" : "");
empty = false;
}
@@ -422,6 +422,7 @@ trk_file_interest_change(const char *trig, struct pkginfo *pkg,

tfi = nfmalloc(sizeof(*tfi));
tfi->pkg = pkg;
+ tfi->pkgbin = pkgbin;
tfi->fnn = fnn;
tfi->options = opts;
tfi->samefile_next = *trigh.namenode_interested(fnn);
@@ -435,7 +436,7 @@ found:
if (signum > 1)
ohshit(_("duplicate file trigger interest for filename `%.250s' "
"and package `%.250s'"), trig,
- pkg_name(pkg, pno_nonambig));
+ pkgbin_name(pkg, pkgbin, pno_nonambig));
if (signum > 0)
return;

@@ -463,7 +464,7 @@ trig_file_interests_update(void)

for (tfi = filetriggers.head; tfi; tfi = tfi->inoverall.next)
fprintf(file->fp, "%s %s%s
", trigh.namenode_name(tfi->fnn),
- pkg_name(tfi->pkg, pno_nonambig),
+ pkgbin_name(tfi->pkg, tfi->pkgbin, pno_nonambig),
(tfi->options == trig_noawait) ? "/noawait" : "");

atomic_file_sync(file);
diff --git a/lib/dpkg/triglib.h b/lib/dpkg/triglib.h
index d816a37..a1c25ff 100644
--- a/lib/dpkg/triglib.h
+++ b/lib/dpkg/triglib.h
@@ -45,6 +45,7 @@ enum trig_options {

struct trigfileint {
struct pkginfo *pkg;
+ struct pkgbin *pkgbin;
struct filenamenode *fnn;
enum trig_options options;
struct trigfileint *samefile_next;
 
Old 12-13-2011, 09:01 AM
Raphael Hertzog
 
Default Review of pu/multiarch/master

Hi,

On Fri, 02 Dec 2011, Raphael Hertzog wrote:
> I saw you merged fixes for almost everything but your fix for the trigger
> part is not complete enough. I pushed again a new patch in
> pu/multiarch/for-guillem (it's also attached).
>
> The fact that the pkgbin pointer is stored in the filetriggers list is a
> bit unsatisfactory as it might become stale when the package is installed
> (i.e. it should in theory be updated to point to the installed pkgbin
> instead of the available one). Though in practice the cases where the
> pkgbin for available will be updated to something else in the same dpkg
> run are close to zero.

Just a ping to remember you that this issue is still open in your current
pu/multiarch/master.

Cheers,
--
Raphaël Hertzog ◈ Debian Developer

Pre-order a copy of the Debian Administrator's Handbook and help
liberate it: http://debian-handbook.info/liberation/


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Archive: 20111213100146.GA10965@rivendell.home.ouaza.com">h ttp://lists.debian.org/20111213100146.GA10965@rivendell.home.ouaza.com
 

Thread Tools




All times are GMT. The time now is 08:23 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org