Summary: | Opening a markings file might give incorrect warning | ||
---|---|---|---|
Product: | [Frameworks and Libraries] libqapt | Reporter: | Vassilis Palassopoulos <palasso> |
Component: | general | Assignee: | Harald Sitter <sitter> |
Status: | RESOLVED WORKSFORME | ||
Severity: | normal | CC: | rohan, sitter |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Ubuntu | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: |
A relatively long markings file
A smaller version of the markings file (bug doesn't appear) A smaller version of the markings file (bug appears) Don't interrupt method when pkgIter.end() becomes true A markings file where not all markings get processed |
Description
Vassilis Palassopoulos
2013-10-04 17:04:24 UTC
Created attachment 82662 [details]
A relatively long markings file
Comment on attachment 82662 [details]
A relatively long markings file
A subset of the markings file I have for installing my preferred applications on a fresh install
Created attachment 82665 [details]
A smaller version of the markings file (bug doesn't appear)
A subset of test-small on which the bug doesn't appear.
Created attachment 82666 [details]
A smaller version of the markings file (bug appears)
Almost the same as test-small-works with the inclusion of one additional package. With that small change, the bug appears
For reasons of reproducibility and testing I attached a set of files (markings files) on which the bug appears (or not): A relatively long markings file: This is a subset of the markings file I have for installing my preferred applications on a fresh system. A smaller version of the markings file (bug doesn't appear): This is a subset of the previous markings file on which the bug doesn't appear. A smaller version of the markings file (bug appears): This is almost identical with the previous markings file with the difference of the inclusion of one additional package install. With that small change, the bug appears. After reading the code of bool Backend::loadSelections(const QString &path) and comparing it with bool RPackageLister::readSelections(istream &in) (http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/raring/synaptic/raring/view/head:/common/rpackagelister.cc) from Synaptic (which uses the same basic algorithm) I think I found the bug inside a while loop: if (pkgIter.end()) { return false; } When pkgIter.end() becomes true, the method interrupts and returns false. What should happen is check when pkgIter.end() becomes false in order to execute the remaining commands in the while loop. In the case that it becomes true it should simply iterate mapIter and the while loop. I'll upload a patch with the fix. Created attachment 82838 [details]
Don't interrupt method when pkgIter.end() becomes true
The aforementioned reason for existence of this bug alongside with the patch that suggests how it could be fixed, suggests that in some cases the method interrupts while partially executing some of the commands (of marking some package changes). After testing it more, I found a use case where this happens (some of the markings in the markings file don't get processed as the method interrupted before they got processed). For that reason this new finding suggests that the bug is more serious than was initially thought. I will upload a markings file with the case that some markings don't get processed. Created attachment 82872 [details]
A markings file where not all markings get processed
Hiya I suppose you're running Saucy? Assuming that you are, then, the adobe-flash-properties-kde package doesn't exist in Saucy ( infact the entire adobe-flashplugin source was not built for saucy ). So the error is actually a valid one since Muon could not mark the adobe-flash-properties-kde package for installation since it doesn't exist. While your patch is fine, I think it would be better to throw a more meaningful error like "Could not mark package foo for installation/removal" rather than just continue with the packages it successfully marked. OTOH this also exposes a bug where libqapt doesn't ignore a line starting with tab. Hi, I tested it in both Raring and Saucy (when it was beta 2) so I didn't notice this package was missing in Saucy. I'd like to mention though that this package might not be the only reason this error arises. Take a look at "A smaller version of the markings file (bug appears)" and "A smaller version of the markings file (bug doesn't appear)". I've checked those packages and they are all available on Saucy. This makes me believe the error is not valid. I tried out http://pastebin.kde.org/prslwawfnwith "A smaller version of the markings file (bug appears)" and it works for me. Could you check if that works for you? Ugh, bugzilla ate a space, the link is supposed to be http://pastebin.kde.org/prslwawfn Before commiting this, I'd like a justification from you as to why you use tab's to add package descriptions instead of using a '#' which is the supported method. I checked again if it works for me. Currently it doesn't work on what ships on Saucy. I can't check if it works with the change because I am having a difficulty when building Muon (it builds against the installed libQApt instead the one I cloned from the git repo and applied the patch, tried to change that with no luck (I'm new with cmake)). For the same reason I wasn't able to test the patch I made. The usage of tabs instead of the usage of '#' was purely for aesthetics. I was previously using Synaptic where it worked just fine and afterwards I found that it works on Muon on some circumstances as well (e.g. the bug doesn't appear file). So if the problem persists only with files that use tabs for comments instead of '#' it's just a cosmetic issue and the only real reason I could see it as important is purely for reasons of interoperability between Synaptic and Muon. At this point I'd like to remind that there is also a command on dpkg that does the same job: dpkg --set-selections < selection.txt I haven't checked if it supports files that use tabs for comments and I don't know in general how interoperable set-selections is with Muon and Synaptic. The more interoperability the better of course. For my personal usage it doesn't matter if the comment has to start with '#' instead of a tab. I tested again my complete markings file (adding '#' at the start of every comment and removing the flash package this time as you suggested (didn't notice this at first)) and it worked as expected. So there are two things 1. Should Muon report which markings it couldn't process, as you suggested, than simply aborting the process with a general message? Should it process the other markings as well in the case it couldn't process a subset of them? 2. Does Muon have it's own standard of markings file format or does it try to be interoperable with other programs? Which ones? Synaptic? dpkg? In any case currently when it pops up the error message, it processes some markings, although it says it couldn't process them (implying it couldn't process any of them) and the GUI behaves awkwardly (as previously explained). Additionally the undo/redo functionality gets messed up a little bit. For example if it tries to read blank markings files, it'll create new states of changes (switching between them with undo/redo) while actually nothing changes! (since the states are void) Since I see the main dev is not available, maybe the best choice is the least time consuming effort. This would be changing the error message to "Could not mark some changes. Please make sure that the file is a markings file created by either the Muon Package Manager or the Synaptic Package Manager". This partially fixes the bug as it warns that only some changes couldn't be marked while the GUI awkwardness persists. Thank you for the bug report. As this report hasn't seen any changes in 5 years or more, we ask if you can please confirm that the issue still persists. If this bug is no longer persisting or relevant please change the status to resolved. I am not a user of the Muon Package Manager any more thus this issue does not affect me anymore. |