Bug#657557: [PATCH] Alternate patch for missing long descriptions
Cyril Brulebois <kibi@debian.org> writes:
> For the sake of getting my fellow packages/translations maintainers to
> get the whole picture since you insist so much on proposing a wrong
> solution and calling mine â??papering over the bugâ??, I'll reply anyway.
Please don't make up quotes I never said. And thanks for finally
responding with more than "wrong" to support your case even if it
doesn't explain why long description in the package db are neccessary or
give an example what remains broken with just my patch.
> Goswin von Brederlow <goswin-v-b@web.de> (22/02/2012):
>> Now my patch (attached) fixes this problem by only computing the
>> description-md5 field if it is missing in Packages.bz2. A simple one
>> line fix. The rest of the code already does all the right things and
>> looks up the translation correclty including falling back to 'en' as
>> needed.
>
> That's only if you're interesting in getting the translations back.
Which is what is done when generating the webpages so this seems rather
important to me.
> Now
> go read the block of code right above $data{'description-md5'}â?¦
Let see:
# we add some additional data here
my $descr = "$data{'description'}\000$data{'package'}\000"
.($data{'tag'}||'');
Description in database format.
my $sdescr = $data{'description'};
$sdescr =~ s/\n.*//os;
Description in text format.
my $did = undef;
if (exists($descriptions{$descr})) {
$did = $descriptions{$descr};
} else {
$did = 1 + $#descriptions;
$descriptions[$did] = $descr;
$descriptions{$descr} = $did;
}
Reuse the same $did if the description already exists.
Ok, so without your patch this bit of code will share a $did if the
short description matches. That certainly differs from what it used to
do. So lets see what effect that has:
----------------- http://localhost/sid/comixcursors -----------------
Package: comixcursors (0.7.2-2)
transitional dummy package
ComixCursors is a set of mouse pointer themes for X11 in the style of comic-book art.
This package is transitional to install the right-handed, translucent cursor set, which is now in the \u2018comixcursors-righthanded\u2019 package.
Tags: Role: Application Data, Dummy Package, X Window System: Theme
Packages providing comixcursors
...
------------- http://localhost/sid/ttf-aoyagi-soseki -----------------
Package: ttf-aoyagi-soseki (20070207-6)
transitional dummy package
This package is a dummy transitional package. It can be safely removed.
Tags: Culture: Japanese, Made Of: Font, Role: Standalone Data, Dummy Package
...
----------------------------------------------------------------------
Oh wait, we always use the description from the translation file even if
it is the english "translation".
>> Correct me if I'm wrong but here is how I understand Cyrils patch: It
>> works by fixing the symptom instead of the problem. In [PATCH 2/4] it
>> checks if the Packages.bz2 file contains an description-md5 field, If
>> so it looks up the english translation for the package and replaces
>> the description with the english translation, thereby restoring the
>> long description for the package (line 146 with his patch).
>
> â?¦ and this is needed so that storing the description in the database
> (what I pointed to above: $descr, $sdescr, etc.) happens properly,
> meaning: the long description, not the short one only.
True. Without your patch the long description is no longer stored in the
package database, only in the translations database.
What you haven't explained is why that is needed. Without your patch the
packages_descriptions.db (ever only used by bin/parse-packages) and
descriptions_packages.db (used in DoShow.pm and Search.pm) will have
bogus entries.
But the package pages show the translation and searching in the
descriptions also seems to properly look into the english translations.
E.g. searching for "It can be safely removed" gives among others:
--- http://localhost/search?searchon=all&keywords=It+can+be+safely+removed ---
...
Package gnash-opengl
sid (unstable) (oldlibs): dummy package for gnash-opengl removal
0.8.10-3: all
----------------- http://localhost/sid/gnash-opengl -----------------
Package: gnash-opengl (0.8.10-3)
dummy package for gnash-opengl removal
This package is a transitional package for gnash-opengl removal.
It can be safely removed when Gnash is installed.
Tags: User Interface: X Window System, Role: Dummy Package, Program, Interface Toolkit: uitoolkit::gtk, use::playing, Supports Format: SWF, ShockWave Flash, Works with: works-with::audio, works-with::video, X Window System: Application
----------------------------------------------------------------------
As you can see the search string only appears in the long description
(meaning the english translation) and not the short description.
So where is the long description required in the package database?
Where is the descriptions_packages.db relevant? How do I get it to do
something wrong?
Or are they all just relicts from before translation support was added?
To me it seems like the unique description id (DID) should be changed
from integer to the description-md5. No need to search all descriptions
for every package again and again to generate the interger DID when we
already have a unique key in the description-md5 for just that use.
Just an optimization? Sure. But that would optimize away your patch or not?
>> And now that the long description has been restored the computation of
>> description-md5 a few lines later computes to the right value, the one
>> that is already present.
>
> As said on IRC, if applied after my patches, you're optimizing out an
> MD5 sum in case it's present already; which doesn't hurt.
I never denied that.
What baffels me though is that you maintain my patch is wrong and at the
same time say it is an optimization.
> But that
> doesn't fix the issue with short descriptions floating around.
What issue is that?
> Mraw,
> KiBi.
MfG
Goswin
PS: BACKEND is out of sync with the rest of the code. E.g.
packages_small.db contains the Description-md5 now. Right where the
Notes suggested "maybe add did right before shortdescription?"
Reply to: