Summary: | Process more complex markings files | ||
---|---|---|---|
Product: | [Applications] muon | Reporter: | Vassilis Palassopoulos <palasso> |
Component: | libqapt | Assignee: | Jonathan Thomas <echidnaman> |
Status: | RESOLVED FIXED | ||
Severity: | wishlist | Keywords: | investigated |
Priority: | NOR | ||
Version: | 2.0.0 | ||
Target Milestone: | --- | ||
Platform: | Ubuntu | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/libqapt/4ae0602d0f81f48f58395966a8bce8e57226cb8e | Version Fixed In: | 2.1.0 |
Attachments: |
Patch to fix the bug
A minor fix on coding style |
Description
Vassilis Palassopoulos
2013-02-06 19:49:41 UTC
After navigating through the code, I find the issue has to do with the way the method: bool Backend::loadSelections(const QString &path) is implemented on backend.cpp on LibQApt (as can be seen here: https://projects.kde.org/projects/extragear/sysadmin/libqapt/repository/revisions/master/entry/src/backend.cpp) It's clear that the reason Muon can only handle the beforementioned strict syntax ( <package_name><tab><tab>install<end_of_line>) on markings files is this line of code: int eqpos = line.indexOf("\t\t"); which basically identifies the position of the <tab><tab> in order to do the separation between package name and action acted upon it (as can be seen on the code that follows): if (eqpos < 0) { // Invalid lineIndex++; continue; } else { aKey = line.left(eqpos); aValue = line.right(line.size() - eqpos -2); } Also it's clear from the following code that the only checks are done for 'i' (install being the sane word expected there in a markings file in order to install the package) and 'd' or 'u' (deinstall or uninstall to deinstall the package) if (aValue.at(0) == 'i') { actionMap[aKey] = Package::ToInstall; } else if ((aValue.at(0) == 'd') || (aValue.at(0) == 'u')) { actionMap[aKey] = Package::ToRemove; } P.S.: So the component affected by this bug is LibQApt (and it persists in 2.0.0 since this part of the code hasn't changed) P.S.2: One can see how Synaptic reads markings files on method: bool RPackageLister::readSelections(istream &in) http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/raring/synaptic/raring/view/head:/common/rpackagelister.cc Created attachment 79936 [details]
Patch to fix the bug
Synaptic can process markings files with a less strict syntax with the method: bool ParseQuoteWord(const char *&String,string &Res) from apt-pkg/strutl.h as can be seen from previous comment.
backend.cpp already imports strutl.h so I decided to create a proposed patch that uses tha same method (providing interoperability as well as code reuse)
Since line, aKey, aValue are QByteArrays I did the following: ParseQuoteWord(line.constData(), aKey.data())
In case this produces any problems maybe aKey, aValue could be defined as Strings or QStrings instead of QByteArrays? line shouldn't have any problem since ParseQuoteWord doesn't change its data.
Also I extended the cases to include 'r' (remove keyword used on synaptic for deinstall) and 'p' (purge) as well case Package::ToPurge
Git commit 4ae0602d0f81f48f58395966a8bce8e57226cb8e by Jonathan Thomas. Committed on 17/05/2013 at 18:31. Pushed by jmthomas into branch 'master'. Support newer Synaptic-style markings file formats, and restrict whitespace requirements a bit. Thanks to Vassilis Palassopoulos for the patch! FIXED-IN:2.1.0 M +17 -11 src/backend.cpp http://commits.kde.org/libqapt/4ae0602d0f81f48f58395966a8bce8e57226cb8e Just noticed a small redundancy that originates from a mistake I did on my patch. I added an additional "(" line 1250: } else if ((aValue.at(0) == 'p')) { Can change to: } else if (aValue.at(0) == 'p') { Created attachment 80020 [details]
A minor fix on coding style
Just removing unneeded parentheses ()
|