Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Mon, Mar 07, 2011 at 02:50:11PM -0800, Steve Langasek wrote:
On Mon, Mar 07, 2011 at 11:33:12PM +0100, David Kalnischkies wrote:
On Mon, Mar 7, 2011 at 20:39, Steve Langasek vor...@debian.org wrote:
Oh man, I can't believe I missed FullName(), that's kind of an obvious
function, isn't it. :) Do you want me to change the patch to use this?
The
No need to, i just manual-merged your patch (bzr errored out on the
try to merge¹) and committed my changes after that as attached.
Nothing fancy, just replaced the handmade Name() + : + Arch() with
FullName() and new[] with strdup as it reduces the linecount a bit.
Great, thanks!
Hope that makes it clear what i meant with FullName() usage.
We could possibly replace the complete if-else with FullName, but then
we would have to strdup for all packages and not just the multiarch ones,
so that bit of duplication is good as it is. :)
Yep, clear.
¹ bzr: ERROR: Revision is not compatible with
KnitPackRepository('file:///…/apt/.bzr/repository/')
… Whatever: too late and too lazy to check that now …
Ah; KnitPack is an old bzr repository format. Assuming your dev environment
is upgraded to squeeze, you should definitely run 'bzr upgrade' on any local
branches you have, as well as on
bzr+ssh://bzr.debian.org/bzr/apt/debian-sid/, to get the much-improved 2a
format. That will bring speed improvements when accessing the repo, and
make everyone happy. :)
Actually, it'd need to be run on bzr+ssh://bzr.debian.org/bzr/apt/apt/ (and
sub directories), which is a shared repository.
bzr+ssh://bzr.debian.org/bzr/apt/debian-sid/ is a
symlink to bzr+ssh://bzr.debian.org/bzr/apt/apt/debian-sid/. I upgraded the
python-apt repository some time ago, if wanted, I can update apt as well.
--
Julian Andres Klode - Debian Developer, Ubuntu Member
See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.
pgpxnh4vQEg48.pgp
Description: PGP signature
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Sun, Mar 6, 2011 at 05:39, Steve Langasek vor...@debian.org wrote: Please find attached a tentative patch for this issue. It's not exactly the most elegant, but it seemed to be the best we can do without changing the libapt ABI. If you prefer a different solution, please let me know and I can give it a try. Sry for not looking at it earlier myself. I am still quiet busy with carnival duties and exams… Anyway, fine for now beside that i would use I-Pkg.FullName() instead of the handmade Name:Arch. Returns also only a std::string… Adding it to the mmap is properly not worthed it as we don't need it in performance critical operations - and while i have no real figures a small apt-cache pkgnames names.log test generates a 472kb file. This would mean if we save the fullname in the mmap we would add ~1 MB to the mmap… (with 41601 packages available currently). Best regards David Kalnischkies -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Mon, Mar 07, 2011 at 07:08:19PM +0100, David Kalnischkies wrote: On Sun, Mar 6, 2011 at 05:39, Steve Langasek vor...@debian.org wrote: Please find attached a tentative patch for this issue. It's not exactly the most elegant, but it seemed to be the best we can do without changing the libapt ABI. If you prefer a different solution, please let me know and I can give it a try. Sry for not looking at it earlier myself. I am still quiet busy with carnival duties and exams… Don't worry about it! It was a nice learning experience in apt internals for me :) Anyway, fine for now beside that i would use I-Pkg.FullName() instead of the handmade Name:Arch. Returns also only a std::string… Oh man, I can't believe I missed FullName(), that's kind of an obvious function, isn't it. :) Do you want me to change the patch to use this? The current patch has the advantage of caching the result of _config-Find(), and since as you say it returns std::string, we still need to take care of memory management for our C strings. So for me I don't see a clear advantage either way, and I'm happy to write it in whatever form you prefer it for inclusion. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developerhttp://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: Digital signature
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Mon, Mar 7, 2011 at 20:39, Steve Langasek vor...@debian.org wrote:
Oh man, I can't believe I missed FullName(), that's kind of an obvious
function, isn't it. :) Do you want me to change the patch to use this? The
No need to, i just manual-merged your patch (bzr errored out on the
try to merge¹) and committed my changes after that as attached.
Nothing fancy, just replaced the handmade Name() + : + Arch() with
FullName() and new[] with strdup as it reduces the linecount a bit.
Hope that makes it clear what i meant with FullName() usage.
We could possibly replace the complete if-else with FullName, but then
we would have to strdup for all packages and not just the multiarch ones,
so that bit of duplication is good as it is. :)
Best regards
David Kalnischkies
¹ bzr: ERROR: Revision is not compatible with
KnitPackRepository('file:///…/apt/.bzr/repository/')
… Whatever: too late and too lazy to check that now …
apt-fullname-and-strdup
Description: Binary data
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Mon, Mar 07, 2011 at 11:33:12PM +0100, David Kalnischkies wrote:
On Mon, Mar 7, 2011 at 20:39, Steve Langasek vor...@debian.org wrote:
Oh man, I can't believe I missed FullName(), that's kind of an obvious
function, isn't it. :) Do you want me to change the patch to use this? The
No need to, i just manual-merged your patch (bzr errored out on the
try to merge¹) and committed my changes after that as attached.
Nothing fancy, just replaced the handmade Name() + : + Arch() with
FullName() and new[] with strdup as it reduces the linecount a bit.
Great, thanks!
Hope that makes it clear what i meant with FullName() usage.
We could possibly replace the complete if-else with FullName, but then
we would have to strdup for all packages and not just the multiarch ones,
so that bit of duplication is good as it is. :)
Yep, clear.
¹ bzr: ERROR: Revision is not compatible with
KnitPackRepository('file:///…/apt/.bzr/repository/')
… Whatever: too late and too lazy to check that now …
Ah; KnitPack is an old bzr repository format. Assuming your dev environment
is upgraded to squeeze, you should definitely run 'bzr upgrade' on any local
branches you have, as well as on
bzr+ssh://bzr.debian.org/bzr/apt/debian-sid/, to get the much-improved 2a
format. That will bring speed improvements when accessing the repo, and
make everyone happy. :)
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
signature.asc
Description: Digital signature
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
tags 614298 patch
thanks
Hi David,
Please find attached a tentative patch for this issue. It's not exactly the
most elegant, but it seemed to be the best we can do without changing the
libapt ABI. If you prefer a different solution, please let me know and I
can give it a try.
--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: steve.langa...@linaro.org-20110306043047-\
# t5brj8ug8jjf8r0j
# target_branch: bzr+ssh://bzr.debian.org/bzr/apt/debian-sid/
# testament_sha1: 7dfd5db21b8c7d51ddd58754958de24db9ff06b0
# timestamp: 2011-03-05 20:33:38 -0800
# base_revision_id: bubu...@debian.org-20110224183912-4l7923vitatsftfg
#
# Begin patch
=== modified file 'apt-pkg/deb/dpkgpm.cc'
--- apt-pkg/deb/dpkgpm.cc 2011-02-03 13:22:32 +
+++ apt-pkg/deb/dpkgpm.cc 2011-03-06 04:30:47 +
@@ -881,7 +881,10 @@
// Generate the argument list
const char *Args[MaxArgs + 50];
-
+ // keep track of allocated strings for multiarch package names
+ char *Packages[MaxArgs + 50];
+ unsigned int pkgcount = 0;
+
// Now check if we are within the MaxArgs limit
//
// this code below is problematic, because it may happen that
@@ -989,13 +992,22 @@
}
else
{
+string const nativeArch = _config-Find(APT::Architecture);
for (;I != J Size MaxArgBytes; I++)
{
if((*I).Pkg.end() == true)
continue;
if (I-Op == Item::Configure disappearedPkgs.find(I-Pkg.Name())
!= disappearedPkgs.end())
continue;
- Args[n++] = I-Pkg.Name();
+ if (I-Pkg.Arch() == nativeArch || !strcmp(I-Pkg.Arch(), all))
+ Args[n++] = I-Pkg.Name();
+ else
+{
+ string const PkgDesc = I-Pkg.Name() + string(:) +
string(I-Pkg.Arch());
+ Packages[pkgcount] = new char[PkgDesc.size()+1];
+ strncpy(Packages[pkgcount++],PkgDesc.c_str(),PkgDesc.size()+1);
+ Args[n++] = Packages[pkgcount-1];
+}
Size += strlen(Args[n-1]);
}
}
@@ -1145,6 +1157,11 @@
sigemptyset(sigmask);
sigprocmask(SIG_BLOCK,sigmask,original_sigmask);
+ /* clean up the temporary allocation for multiarch package names in
+ the parent, so we don't leak memory when we return. */
+ for (unsigned int i = 0; i pkgcount; i++)
+delete [] Packages[i];
+
// the result of the waitpid call
int res;
int select_ret;
=== modified file 'debian/changelog'
--- debian/changelog2011-02-24 18:39:12 +
+++ debian/changelog2011-03-06 04:30:47 +
@@ -1,10 +1,16 @@
apt (0.8.11.6) UNRELEASED; urgency=low
+ [ Christian Perrier ]
* Fix error in French translation of manpages (apt_preferences(5)).
Merci, Rémi Vanicat. Closes: #613689
* Complete French manpage translation
* Italian translation update (Milo Casagrande). Closes: #614395
+ [ Steve Langasek ]
+ * apt-pkg/deb/dpkgpm.cc: make sure that for multiarch packages, we're
+passing the full qualified package name to dpkg for removals.
+Closes: #614298
+
-- Christian Perrier bubu...@debian.org Fri, 18 Feb 2011 05:53:49 +0100
apt (0.8.11.5) unstable; urgency=low
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbDU4WsAAwPfgFpwef///38n
TAq+YAbLtvugBoN1W6OgcqUKhoUaaamg0CeKeU9Tyjaj1A0HqPUBoAyaBmoJSQ0yp+hR40Ig
epoGamgAGg4yZNGIaaGAmhiaNMmIGRhNGmmEGTCRQKapvSemin6o8BT1Mmymh6gB6gMjQB6g
yDjJk0YhpoYCaGJo0yYgZGE0aaYQZMJIiMgjEYgmTUzIVPyKfimk9TPUjEaAHqaeUWQAWQE22+g1
GkSTGcmQO9KIFWfHoJiyTZqKNND2dxL2BqkkxVPmpAIqroGzrZekpMpeY2+iAkNooRSChP67GdF7
fVihluNZ0GMabTazeM+qP1O/dkzxhIiMJZnnqJzlONZLUjDTQ8l2UaDwet8ShtMLFQhyC2uLXVqP
SNxlIMz7Z5xhNIwwrUz2S6LVG0g01BQt4tbS+EbBmT3GFFC78eZXQqAY1HzcbHNqjK7f2HYe6x18
BjZr07A9qXwREM/YbwoW13RwDHjhVO7FE5MerBkbYuzONNLRLuvOMiKZ+R3Q8Y54Aqog/9UV05Ma
veBaUP6s3lr5VUWI91y5JWb0FodoDNSeG35HOzeRc+ZVjYdD0Ms60s2AiD/HfFhWHZRBCah5/tIo
gQHAdogeoHUJgIOfMqkfYwwUKweoXAty0FErG8APzrRDB01Z3aJ99Ns0JzkeNXdRNCpR5UytC4rk
Qbc9RVTn+hWT2BDaFdVXv0bY+vMwFgqDhmEN06TWOXm3dfrImJs2bNOQ9gQ0CsQn4FkRb5k4hrrC
lOG7QWlJpIOZYl1pcE8cVCCNxNh7uDk2pAsQPc1R3aP+xK1kWA/GJUFtmiVDUJwk4PkMF8gM74kA
J4ziQbDaqRRMQ0rRq4BCyweKMyik1aZGmm6cct79oGOfIQ4wJyOHNdRY0qA7cOlBW6krfAOLFgua
3SSvpNNOu9gJjbTVVCEecyk6m13BcA2gI51FC5kSeBWSjjpgPMZsLGcGxlRSnpot1KcW02TKu8Cu
4DACvTEt0zqCMNaKGKMNLsX0QZVsxcZGByNZw6ZVamT6BrhN+vXRrjSwSv04NTErgFLg0mioTRKb
ea9CTuPyQY9D2TpfFo8jSMe82ooz5ncL0O31F1g07UFUTWsItHLzlxRNt/i34SsCfPuVArF72FcG
etTh3DdIH2hjAT7f6YxVIgIxWJXUlrnSYCloEZyi2igpGgWoXNcxuQrHZmbFbeiHi8JEUCw9DbQT
LlsrxHhhMFEQZeqQUgX1kEIuBwwoFrEfDAF2CE6hhlvVtIv9J6kGewV/uR7DqRpD5GMCsViB7UiB
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
severity 614298 important thanks Helau (or any other local carneval battle cry)! On Sun, Feb 20, 2011 at 22:48, Steve Langasek vor...@debian.org wrote: Feel free to downgrade the bug severity if you think that's appropriate; but this is definitely a bug that needs to be fixed before we can roll out multiarch. :-) As it blocks testing transition and only breaks a usecase which can be exercised only with a non-released version of dpkg i think important is better suited. Its grave for multiarch, but given that my last information from the beginning of february is that dpkg maintainers are discussing how the commandline interface will be for specifying which architecture of the package is meant and as i don't know what the outcome of this discussion is i hope its relatively reasonable that i haven't implemented it so far. ;) Best regards David Kalnischkies P.S.: I am still not too sure what to do with pkg:all + MultiArch flag. In general, i would appreciate if it could be added to the spec in more detail as aptitude/cupt/whatever will hit the same problem at the time they implement it… -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
On Mon, Feb 21, 2011 at 06:20:03PM +0100, David Kalnischkies wrote: On Sun, Feb 20, 2011 at 22:48, Steve Langasek vor...@debian.org wrote: Feel free to downgrade the bug severity if you think that's appropriate; but this is definitely a bug that needs to be fixed before we can roll out multiarch. :-) As it blocks testing transition and only breaks a usecase which can be exercised only with a non-released version of dpkg i think important is better suited. Ok. :) Its grave for multiarch, but given that my last information from the beginning of february is that dpkg maintainers are discussing how the commandline interface will be for specifying which architecture of the package is meant and as i don't know what the outcome of this discussion is i hope its relatively reasonable that i haven't implemented it so far. ;) I think it's clear that $pkg:$arch is the correct way to unambiguously specify a foreign-arch package on the command line to dpkg. The only open point of discussion is how to interpret package names when they *don't* have an architecture specified. So I don't think there's any risk that you will need to reimplement this differently later. P.S.: I am still not too sure what to do with pkg:all + MultiArch flag. In general, i would appreciate if it could be added to the spec in more detail as aptitude/cupt/whatever will hit the same problem at the time they implement it… I'm definitely happy to expand on the spec to add any details that you think are missing. Can you tell me what it is that should be expanded on here? I think the current language about Architecture: all packages will be treated as equivalent to packages of the native architecture for all dependency resolution is pretty unambiguous, and I've corrected the problem that we were saying Multi-Arch wasn't allowed for Architecture: all packages. What else should I add here? (Sorry, I'm probably suffering from author's bias here where I think what I've written is perfectly clear when it isn't clear at all. :) -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developerhttp://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: Digital signature
Bug#614298: multiarch: apt-get remove of foreign-arch package removes wrong one
Package: apt Version: 0.8.11.3 Severity: grave User: vor...@debian.org Usertags: multiarch Feel free to downgrade the bug severity if you think that's appropriate; but this is definitely a bug that needs to be fixed before we can roll out multiarch. :-) # apt-get remove libselinux1:i386 Reading package lists... Done Building dependency tree Reading state information... Done The following packages will be REMOVED: libselinux1:i386 0 upgraded, 0 newly installed, 1 to remove and 84 not upgraded. Need to get 0 B/764 kB of archives. After this operation, 6214 kB of additional disk space will be used. Do you want to continue [Y/n]? Preconfiguring packages ... (Reading database ... 28974 files and directories currently installed.) Removing libselinux1 ... dpkg: libselinux1: dependency problems, but removing anyway as you requested: libglib2.0-0 depends on libselinux1 (= 1.32). util-linux depends on libselinux1 (= 1.32). sysvinit-utils depends on libselinux1 (= 1.32). dpkg depends on libselinux1 (= 1.32). libpam-modules-bin depends on libselinux1 (= 1.32). sysvinit depends on libselinux1 (= 1.32). passwd depends on libselinux1 (= 1.32). sed depends on libselinux1 (= 1.32). mount depends on libselinux1 (= 2.0.15). libpam-modules depends on libselinux1 (= 2.0.85). coreutils depends on libselinux1 (= 1.32). Removing libselinux1 ... dpkg: error while loading shared libraries: libselinux.so.1: cannot open shared object file: No such file or directory Processing triggers for libc-bin ... ldconfig deferred processing now taking place /usr/bin/dpkg: error while loading shared libraries: libselinux.so.1: cannot open shared object file: No such file or directory E: Sub-process /usr/bin/dpkg returned an error code (127) # So, apt told dpkg to remove the wrong package. The commandline passed is: /usr/bin/dpkg --status-fd 15 --force-depends --force-remove-essential --remove libselinux1 It should be asking for the removal of libselinux1:i386 instead. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developerhttp://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: Digital signature
