Bug 12766 - Existence of backports packages causes problems in rpmdrake
: Existence of backports packages causes problems in rpmdrake
Status: RESOLVED FIXED
Product: Mageia
Classification: Unclassified
Component: RPM Packages
: 4
: x86_64 Linux
: Normal Severity: major
: ---
Assigned To: Thierry Vignaud
:
:
:
:
: 13606
: 10218 12170 12705 13039 13156
  Show dependency treegraph
 
Reported: 2014-02-14 16:30 CET by James Kerr
Modified: 2015-02-19 14:02 CET (History)
10 users (show)

See Also:
Source RPM: rpmdrake-6.10-1.mga4.src.rpm
CVE:


Attachments
patch to partially fix the problem (1.72 KB, patch)
2014-05-14 22:13 CEST, Angelo Naselli
Details | Diff
patch to solve the problem (3.17 KB, patch)
2014-05-15 10:16 CEST, Angelo Naselli
Details | Diff
patch that solves the problem (3.13 KB, patch)
2014-05-15 16:31 CEST, Angelo Naselli
Details | Diff
Patch to fix the compare package issue (1.12 KB, patch)
2014-05-17 21:31 CEST, Angelo Naselli
Details | Diff
gui.pm to test (42.20 KB, application/x-perl)
2014-06-02 20:30 CEST, Angelo Naselli
Details
Result of using Find box (105.90 KB, image/png)
2014-06-02 20:42 CEST, James Kerr
Details
Results from updates filter (73.61 KB, image/png)
2014-06-02 20:45 CEST, James Kerr
Details
keep only latest package, but per medium (531 bytes, patch)
2014-06-27 09:21 CEST, Thierry Vignaud
Details | Diff
keep only latest package, but per medium (v2) (596 bytes, application/octet-stream)
2014-06-27 10:31 CEST, Thierry Vignaud
Details

Description James Kerr 2014-02-14 16:30:31 CET
If libreoffice is not installed rpmdrake displays only four libreoffice packages
(installed libreoffice packages are listed)

Remove libreoffice
Search for libreoffice
Only 4 relatively unimportant libreoffice packages are listed

"urpmi libreoffice" installs the entire libroffice suite, so I think that this may not be a mirror or hdlist issue 

https://forums.mageia.org/en/viewtopic.php?f=7&t=6996

There may be other packages similarly affected, but I've not identified them, yet.

Reproducible: 

Steps to Reproduce:
Comment 1 Manuel Hiebel 2014-02-14 17:37:56 CET
Indeed confirmed, could be a related that libreoffice is in backport ?
and urpmq -ya libre show them all


(btw if I go with the dropdown menu in the backport pkgs are here without enabling these medias)
Comment 2 James Kerr 2014-02-14 18:53:13 CET
(In reply to Manuel Hiebel from comment #1)

>>could be a related that libreoffice is in backport
Could be. There have been strange problems with backports in the (Mandriva) past. We've never seen any in Mageia, because we never had backports before :)

and your supposition is correct. I renamed /core/Backports as /core/Bckports
and /core/Backports/Testing as /core/Bckports/Testing and libreoffice packages are now all displayed in rpmdrake.


>> (btw if I go with the dropdown menu in the backport pkgs are here without enabling these medias)

That is by design. mgaonline updates all backports repo's, whether or not they are enabled, so that this can be done.
Comment 3 Andrew Brewster 2014-02-17 11:06:43 CET
In a fresh installation libreoffice-core, -headless, -javacommon, -langpack-{language-code}, -ure, from version 4.2.0.4 are shown as installed in rpmdrake. Also shown, but not as installed are -presentation-minimizer, and  -voikko, but of version 4.1.3.2

running 'urpmi libreoffice' gives
The following packages can't be installed because they depend on packages
that are older than the installed ones:
libreoffice-impress-4.1.3.2-4.mga4
libreoffice-4.1.3.2-4.mga4
Continue installation anyway? (Y/n) n

If you remove all libreoffice packages in rpmdrake, then only 6 packages from version 4.1.3.2 are shown.

running 'urpmi libreoffice' then gives

To satisfy dependencies, the following packages are going to be installed:
  Package                        Version      Release       Arch    
(medium "Core Release (Mageia4DVD1)")
  lib64cdr0                      0.0.14       3.mga4        x86_64  
  lib64cmis0.3_3                 0.3.1        5.mga4        x86_64  
  lib64mspub0                    0.0.6        3.mga4        x86_64  
  lib64mwaw1                     0.1.11       2.mga4        x86_64  
  lib64odfgen0                   0.0.2        4.mga4        x86_64  
  lib64orcus0.6_0                0.5.1        9.mga4        x86_64  
  lib64visio0                    0.0.31       3.mga4        x86_64  
  lib64wpd0.9_9                  0.9.9        2.mga4        x86_64  
  lib64wpg0.2_2                  0.2.2        2.mga4        x86_64  
  lib64wps0.2_2                  0.2.9        4.mga4        x86_64  
  libreoffice-base               4.1.3.2      4.mga4        x86_64  
  libreoffice-calc               4.1.3.2      4.mga4        x86_64  
  libreoffice-core               4.1.3.2      4.mga4        x86_64  
  libreoffice-draw               4.1.3.2      4.mga4        x86_64  
  libreoffice-graphicfilter      4.1.3.2      4.mga4        x86_64  
  libreoffice-headless           4.1.3.2      4.mga4        x86_64  
  libreoffice-impress            4.1.3.2      4.mga4        x86_64  
  libreoffice-langpack-en        4.1.3.2      4.mga4        x86_64  
  libreoffice-math               4.1.3.2      4.mga4        x86_64  
  libreoffice-ogltrans           4.1.3.2      4.mga4        x86_64  
  libreoffice-opensymbol-fonts   4.1.3.2      4.mga4        noarch  
  libreoffice-pdfimport          4.1.3.2      4.mga4        x86_64  
  libreoffice-ure                4.1.3.2      4.mga4        x86_64  
  libreoffice-writer             4.1.3.2      4.mga4        x86_64  
  lpsolve                        5.5.2.0      5.mga4        x86_64  
  postgresql-jdbc                9.2.1002     6.mga4        noarch  
(medium "Core Release (distrib1)")
  libreoffice                    4.1.3.2      4.mga4        x86_64  
  libreoffice-java-common        4.1.3.2      4.mga4        x86_64  (suggested)
311MB of additional disk space will be used.
94MB of packages will be retrieved.
Proceed with the installation of the 28 packages? (Y/n) 

note the version, which is from the installation DVD, from which I've created a local files medium. All the packages were installed from the local medium, except libreoffice, and libreoffice-java-common, which were installed form a mirror site.

Perhaps this is a case of the two missing packages from the installation DVD causing the problems.
Comment 4 James Kerr 2014-02-17 12:57:55 CET
(In reply to Andrew Brewster from comment #3)
>> In a fresh installation libreoffice-core... from version 4.2.0.4 are shown as >> installed in rpmdrake.

What installation method did you use? The question is how libreoffice 4.2 came to be installed in a fresh installation. This should not have happened.

Version 4.2 of libreoffice is only available in the /backports_testing repo and should not have been installed unless you specifically selected to install it in preference to the release version. backports and testing packages should never be installed by default, unless those repo's are explicitly enabled by the user.
Comment 5 James Kerr 2014-02-17 13:43:28 CET
I've just realised that there is potentially a design flaw in the way in which backports are displayed by rpmdrake. Selecting backports in the first filter will display packages from /backports_testing. It perhaps should be restricted to displaying packages from the /backports repo's. 

This was designed originally for Mandriva, where there were no /backports_testing repo's and so the design probably did not contemplate the possibility of such.


I've changed the subject of the bug to more accurately reflect the issues involved. There may be others. IIRC there may have been open bugs about the handling of backports in the GUI applications at the time of the fork. I don't know if anything will have been done about those since the fork.
Comment 6 Manuel Hiebel 2014-02-23 21:21:07 CET
btw the way this doesn't happend after removing/adding media 
cf bug 12859

(and maybe if the users never enable the backport media)
Comment 7 Rémi Verschelde 2014-03-10 11:14:12 CET
Bug confirmed on Mageia 4 i586. The presence of dkms-bbswitch 0.8 in core/backports_testing prevents rpmdrake from finding the core/release 0.4.2 version.
urpmq -i dkms-bbswitch find it though.
Comment 8 James Kerr 2014-03-10 12:47:11 CET
It would probably be best if no packages were added to /backports_testing repo's until this problem is resolved. tmb has removed the libreoffice packages which caused the first report.
Comment 9 Guillaume 2014-04-21 12:03:50 CEST
Hi there !
Any news about the problem ?
Comment 10 Manuel Hiebel 2014-04-23 18:22:03 CEST
The original bug, with interesting view is https://qa.mandriva.com/show_bug.cgi?id=40556

looks something is broken since
Comment 11 Manuel Hiebel 2014-04-23 19:20:37 CEST
It's because of http://gitweb.mageia.org/software/rpmdrake/commit/Rpmdrake?id=d600679cdaf402a0f97bbbd76e7fa04c06c5a37e

that we have added to not list all version of rpm in updates
Comment 12 Rémi Verschelde 2014-05-11 15:03:28 CEST
Adding some perl developers in CC, it seems like Manuel found the culprit in comment 11, we just need someone to fix it :-)
Comment 13 Angelo Naselli 2014-05-11 18:09:01 CEST
Defining me now as a perl developer, sounds really funny... 
If tv is the author of that commit i think he is able to explain why, and fix this bug.

I don't think that reverting a commit without knowing either why or the well the code is the solution.
Comment 14 Rémi Verschelde 2014-05-11 19:46:20 CEST
Well by fixing I didn't meant reverting it. And if you don't know the code at all I'm not trying to force this bug on you, I'm just trying to get it fixed.
Comment 15 Angelo Naselli 2014-05-11 20:12:49 CEST
Well then wonder why tv has not said anything about this bug yet? 

I also do want backports to be fixed, but this bug should also be valid if you add your own repository for updates, i understand that this situation should not be considered as maintained, but a bug anyway. So maybe we can find some other point that can bring more interest in who are really able to fix it.

@Rémi I see your point though rpmdrake has been studied a bit, trying to port it to adminpanel, but that is another story... :) 
@Rémi i was aware of this bug anyway ;)
Comment 16 Angelo Naselli 2014-05-13 14:30:37 CEST
@Thierry I could be wrong, and i read the code quickly, but i think the problem is not where Manuel pointed but some rows down, e.g.
foreach my $medium (@backport_medias) {
        update_pbar($gurpm);

        # The 'searchmedia' flag differentiates inactive backport medias
        # (because that option was passed to urpm::media::configure to
        # temporarily enable them)

        my $backports =
            $medium->{searchmedia} ? \@inactive_backports : \@active_backports;

      foreach my $pkg_id ($medium->{start} .. $medium->{end}) {
          next if !$pkg_id;
          my $pkg = $urpm->{depslist}[$pkg_id];
          $pkg->flag_upgrade or next;
          my $name = $pkg->fullname;
          push @$backports, $name;
          $all_pkgs{$name} = { pkg => $pkg, is_backport => 1 };
      }
    }

It seems no appsumptions to add backport packages to all_pkgs is done, so they are added overwiriting the previous fullname.
But i could be wrong, of course.
Comment 17 Angelo Naselli 2014-05-13 14:33:31 CEST
hmm i missed $pkg->flag_upgrade or next;
Comment 18 Angelo Naselli 2014-05-13 23:02:49 CEST
@Remi, how can i test this issue? Should i enable backport repositories? are they available yet or they've been disabled?
Comment 19 Angelo Naselli 2014-05-14 00:09:01 CEST
If you remove the line (Comment #16) push @$backports, $name; then you will find dkms-bbswitch, but the version is 0.8, e.g. the backport one.

So yes an error is in commit on Comment #11, because the backport package is there yet and no tests are performed to check if enabled or not.
I think you have more experience than me to fix that and better investigate the reason
Comment 20 Angelo Naselli 2014-05-14 22:13:03 CEST
Created attachment 5158 [details]
patch to partially fix the problem

This patch partially fix the problem, now bp packages are added only if they are in a enabled media.
Though that does not solve all the problems because if
1) a package is in BP but not in core it is shown anyway, despite of if it is enabled or not (because the key $l{$key} is not found)
2) the bp media is enabled to find bp pckages explicit BP media filter selection it's needed

A better way should be found, because I'm not sure if the order in which the packages are found is important or not, I haven't investigated a lot.
Comment 21 Angelo Naselli 2014-05-15 10:16:50 CEST
Created attachment 5160 [details]
patch to solve the problem

This patch should definitely solve this issue, filters were wrong after the change of the new way of presenting all the packages (just last version), now a check if backport is enabled is also performed to eventually add the greater packages in the list
Comment 22 Marja van Waes 2014-05-15 10:49:31 CEST
(In reply to Angelo Naselli from comment #21)
> Created attachment 5160 [details]
> patch to solve the problem
> 
> This patch should definitely solve this issue, filters were wrong after the
> change of the new way of presenting all the packages (just last version),
> now a check if backport is enabled is also performed to eventually add the
> greater packages in the list

Thx for your work, Angelo

It would be nice to have a package to test the fix :-)
Comment 23 Angelo Naselli 2014-05-15 11:01:17 CEST
i have to check a thing yet, i tried to removed the packaged that are only in backports, thay cannot be compare, since they are only there and are added anyway.
but that change, rose again the reported problem.

I will commit the change when i'm confident but thests are welcome.
Comment 24 Angelo Naselli 2014-05-15 11:02:47 CEST
hmm i meant "i tried to remove the packages"
Comment 25 Angelo Naselli 2014-05-15 16:31:37 CEST
Created attachment 5161 [details]
patch that solves the problem

This patch should really fix the problem.

For reviewers, i changed
$l{$key}->compare($pkg) 
to
$l{$key}->fullname lt $pkg->fullname
I hope it is right to understand if a package is greater. The problem with compare
was that retruned 1 for the most and so the package was always added.
Comment 26 Mageia Robot 2014-05-15 16:34:02 CEST
commit 5b24d6f61e17bffc2c9283a65100a29d6a05e586
Author: Angelo Naselli <angelo.naselli@...>
Date:   Thu May 15 16:32:17 2014 +0200

    package list contains only latest updates, not all, so
    filters and backport list have been managed accordingly (fix mga#12766)
---
 Commit Link:
   http://gitweb.mageia.org/software/rpmdrake/commit/?id=5b24d6f61e17bffc2c9283a65100a29d6a05e586
Comment 27 Thierry Vignaud 2014-05-15 21:53:38 CEST
You could have waited a little between posting & committing your patch.
This looks overly complex
Replacing ->compare() call by lt test is nonsense.

The comment about "active backport" is no sense since we're iterating over backports.

Please explain your rationale instead...

Also as rule of thumb, we usually split our patches into smaller pieces with changelogs. Please remember that next time.
Comment 28 Angelo Naselli 2014-05-15 22:13:12 CEST
FWIW you can revert it. I don't mind it's just three months that we are waiting for a comment on this bug.
Comment 29 Marja van Waes 2014-05-15 23:01:15 CEST
(In reply to Thierry Vignaud from comment #27)
> You could have waited a little between posting & committing your patch.

Sorry, but "a little" might mean something very different to you than to Angelo. How long is "a little"?
Comment 30 Angelo Naselli 2014-05-15 23:09:34 CEST
Anyway the technician that is in me, will answer and wait for clarifications.

> The comment about "active backport" is no sense since we're iterating over 
> backports.
Well it was not clear to me, that $medium->{searchmedia} means
inactive backports, but:
 my $backports =
            $medium->{searchmedia} ? \@inactive_backports : \@active_backports;
Seems to say that, so yes "active backport" (I admit two words more could be more
helpfull) shoud recal that code is significant only for active backports and not for incative. Yes we are iterating over backport, but the could be disabled!

> This looks overly complex
This what? the patch? the commit after posting? maybe some words here to explain...

> Replacing ->compare() call by lt test is nonsense.
Ok i take as a costructive advice, since compare would add also the updates on the list, with the side effect of having both, and only if backoport is enabled,
it tried to figure another way to "compare"... you could explain better what is nonsese and maybe why compare is better... I could learn maybe.

>Also as rule of thumb, we usually split our patches into smaller pieces with 
> changelogs. Please remember that next time.
 NEWS            |  5 +++++
 Rpmdrake/gui.pm |  5 ++++-
 Rpmdrake/pkg.pm | 38 ++++++++++++++++++++++++++++----------
 3 files changed, 37 insertions(+), 11 deletions(-)
Well i know greater commits, i don't really understand what you mean...
Comment 31 Thierry Vignaud 2014-05-16 06:51:47 CEST
(In reply to Marja van Waes from comment #29)
> Sorry, but "a little" might mean something very different to you than to
> Angelo. How long is "a little"?

More than 3mn (the difference between 2014-05-15 16:31:37 & 2014-05-15 16:34:02)

(In reply to Angelo Naselli from comment #30)
> This what? the patch? the commit after posting? maybe some words here to
> explain...

I would have reconsidered the "show only latest updates" first.
 
> > Replacing ->compare() call by lt test is nonsense.
> Ok i take as a costructive advice, since compare would add also the updates
> on the list, with the side effect of having both, and only if backoport is
> enabled,
> it tried to figure another way to "compare"... you could explain better what
> is nonsese and maybe why compare is better... I could learn maybe.

This is a logic flaw.
You replace the right way to compare EVR by a string comparison.
This is NOT how EVRs are compared.
Just run perl -e 'warn "1.3-4" lt "1.3-5", warn "1.3-14" lt "1.3-5"'
This is totally wrong!

> Well i know greater commits, i don't really understand what you mean...

From your comments, your changes to Rpmdrake/gui.pm are unrelated and should have been a first preparatory patch

Moving "my @installable_pkgs = map { my $n = $_->fullname; $all_pkgs{$n} = { pkg => $_ }; $n } values %l;+ undef %l;" could have been done in another commit

...
Comment 32 Marja van Waes 2014-05-16 08:12:00 CEST
(In reply to Thierry Vignaud from comment #31)
> (In reply to Marja van Waes from comment #29)
> > Sorry, but "a little" might mean something very different to you than to
> > Angelo. How long is "a little"?
> 
> More than 3mn (the difference between 2014-05-15 16:31:37 & 2014-05-15
> 16:34:02)
> 

24 hours?
Comment 33 Marja van Waes 2014-05-16 08:19:12 CEST
(In reply to Marja van Waes from comment #32)

> 24 hours?

@ tv and Angelo

Only trying to avoid that Angelo's next commit will be after waiting more than 3 hours, to then find out that that is still too little.

(And, if we are ever going to have a wiki page for developers about how to help with our drak* stuff, then it would be nice to have an exact time in that page, too)
Comment 34 Angelo Naselli 2014-05-16 08:53:57 CEST
I'll try to summarize and answer to all, trying to avoid this bug is going to be polemic  

@Marja well, just because i posted the patch here you and TV are right, i could have expected a little bit more. But just because it's me... because there is no so many bugs in witch devs post the patch first. 
Just one note, though, i talked in chat before committing...
Anyone can be wrong, if you don't want to have commits, than add a way with a 
pull request like in github, then you have to decide who is developer and who 
is a contriubutor. So no needs to ask for minutes, hours or days, just look at the pul requests, but also *when* they are served...

@TV, concerning lt you are right, even if compare is wrong there also imo, at last in my tests i've seen inserting a package twice.

@TV concerning to riconsider the "show only latest updates", it isn't up to me,
I thought if you added that you had good reasons, I just understood you haven't done all the changes accordingly, and worked in that direction. Once you said that
we have to make our known stuff working not rewrite them... so I just applied that
rule, and you could also have answered to about *five* days ago comment #14 and comment #15, maybe i would have worked on riconsidering that...

@TV concerning the commit of gui.pm, has been changed for this bug and it *is* related, without that change you won't see backports, and that's why the reporter could not see libreoffice or Remi dkms-bbswitch, just because those packages were inserted and removed at view time. From my point of view a single push for a fixing it's easier to be backported for a patch in stable packages. But i don't mind if you prefer, i can make thousand commits... if i ever decide to contribute more
Comment 35 Angelo Naselli 2014-05-16 08:55:18 CEST
Sorry comment #13 not 14
Comment 36 Colin Guthrie 2014-05-16 12:18:20 CEST
Just to try and stop this bug descending into a bikeshed :p here are my €0.02 I hope I'm being fair, but feel free to pull me up if you think otherwise!


1. FYI, there are ways to do "pull requests" with our git setup, but this isn't really documented much. Ultimately you can push your changes onto your own branches. You have two choices, either a "topic" branch that you intend to be worked on further by others or your own user-specific branch that only you can push too. Simply call your branch "topic/foo" or "user/$USERNAME/foo" and you can push this branch happily to the repo. Others can then work on merging it to master or reworking it accordingly as needed. This is typically useful for > 1 patch however. For a single patch it might make more sense to attach it here as was done and await review (and perhaps prod when no review has happened). I know that such attachments can sit and bitrot here so perhaps more focus is needed on that front. I know people have suggested reviewboard in the past but it's not the easiest to deploy. Perhaps bugzilla extensions for patch review as used on Gnome bugzilla might be an easier option?


2. Regarding Angelo's push, I agree that sometimes that's needed to get some movement on a problem (this certainly got Thierry's attention!), but in this case the patch was probably pushed a prematurely (only a few minutes after posting it as an attachment - as the commit message correctly mentioned the bug number a link, the posting of the attachment was somewhat redundant as the gitweb link is added automatically). I don't think there is an exact time for how long something should be left for review, but 24h is far too low (we all have lives to lead!). I know it can be frustrating waiting for others' input on things when they don't respond, but I suspect Thierry receives even more automated mail than me and it gets overwhelming at times! I think a few gentle prods via the bug and perhaps via direct email would have been better than the "I don't know why ... has not responded" type comments - there are many reasons for not responding and, as mentioned, we all have lives to lead. I often forget things and have a massive pile of mails that need my attention but it'll ultimately take someone pinging me on IRC or sending a direct mail to bring some of those things to the fore. I suspect the same is true with Thierry! I know that the bug currently being worked on is the most important bug to the person doing the work, but not everyone can share that prioritisation :D

Ultimately, sometimes pushing a fix is the way forward, but it really should only be done several days after having posted a patch for review - this way if some later feedback is received after pushing it, there is no reason to complain that the patch submitted without waiting for review! It's essentially covering your back, which is something I like to do frequently!


3. Regarding Thierry's feedback on some parts, I appreciate Angelo is likely annoyed at the lack of progress on the bug, but IMO that's not a justification to respond to some parts of the code-review quite so defensively. The parts about committing the GUI change first makes sense IMO - yes that change is needed for the overall fix to work, that's why TV suggested that it should have been committed first, with a NEWS entry for it, and then the rest of the fix would have gone after that. However, *this is not a big deal*!! :) It's just a suggestion that things can be split up to logical parts which is generally nicer. I don't think he was suggesting it wasn't needed for the overall fix, just unrelated to the code changes... (i.e. it's just that it could be isolated and committed first in a preparatory patch). Smaller changes are generally much easier to read and review. This was just passing comment about the preferred way of doing commits and I think it was an overreaction to suggest doing a thousand commits (I suspect this reaction was stemming from other frustrations rather than this particular comment tho').

I've had this kind of feedback from both Thierry and other people on other projects I've contributed to. It's important to accept things and be relaxed about it. It's almost never a personal attack and regardless of wording and language involved, remember that it's not a slight against you. Try and take feedback with good grace :)

Hope that helps!
Comment 37 Angelo Naselli 2014-05-16 14:17:24 CEST
Well what really annoyed me was the comments, nonsense here nonsense there... without a real explanations.
There are always and ways to tell the things, we are all volunteers and even if
that does not mean we can do anything would also mean we need to take that in
account.
Moreover I was involved here as developer, so i felt like i was...

Just a note, i wasn't aware that commits would have been added here, otherwise i would have not added the patch and just let the commit message without any time 
shift and upload :p 

Now i want to say I am not frustrated and either angry, or under attack.
I just only want to know what i should do now... giving up, change the code
according to the "suggestions"... let me know, because it's not clear to me
Comment 38 Angelo Naselli 2014-05-17 21:31:14 CEST
Created attachment 5166 [details]
Patch to fix the compare package issue

This patch use URPM::Package::compare_pkg instead of lt, wrongly used in my patch to substitute the either wrongly used URPM::Package::compare
Comment 39 Mageia Robot 2014-05-17 21:53:19 CEST
commit 4b19e5f345e39ed8c3350e6acc7179d26c6e4526
Author: Thierry Vignaud <thierry.vignaud@...>
Date:   Sat May 17 21:30:20 2014 +0200

    (compare) croak if used on URPM::Package object (mga#12766)
    
    suggests to use compare_pkg instead
    
    (needs a new rpmdrake)
---
 Commit Link:
   http://gitweb.mageia.org/software/rpm/perl-URPM/commit/?id=4b19e5f345e39ed8c3350e6acc7179d26c6e4526
Comment 40 Angelo Naselli 2014-05-24 15:15:55 CEST
> Angelo Naselli 2014-05-17 21:31:14 CEST

> Created attachment 5166 [details]
> Patch to fix the compare package issue

For those who were interested in testing my patches i've built a package for
mga4 here:
http://www.linux.it/~anaselli/Backport/mga4/rpmdrake-6.10-1.0.mga4.noarch.rpm
Comment 41 claire robinson 2014-05-25 00:19:04 CEST
I'm happy to take part in testing. I haven't followed the discussion though, is this thought to be the correct fix for this now?
Comment 42 Angelo Naselli 2014-05-25 09:54:56 CEST
on comment #41, well here the problem seems to be solved. I installed my package about one week ago, to see if i have regressions, and i seem not to.
But of course I'm not an rpmdrake, URPM, etc... developer, but i think i gave the
evidence -using devel mailing list also- i haven't worked on this problem just to
commit something...

I would have committed and provided an official testing package, but after my first commit i felt a bit disappointed on the way I received feed backs. It's me
maybe I'm wrong but it's me :)

So if someone would like to test and see if it works, we can go on, or letting more trusted developers deciding how to revert, change, or whatever they want to fix this bug.
Comment 43 James Kerr 2014-05-25 12:17:34 CEST
I intend to test this package, but I don't expect to be able to until next weekend. Since there are at present no backports, only testing backports, I will have to make some arbitrary changes to my local mirror in order to test in as real an environment as possible.
Comment 44 claire robinson 2014-05-25 19:12:35 CEST
I haven't been following Angelo, sorry, just caught up with this one when I saw your comment 40 on the bugs ML. I'm sure nobody would cause intentional hurt though.

I imagine there is some frustration at the moment with nobody leading the charge. Hopefully before long we'll get a proper dev team set up and a leader elected who can monitor this kind of thing and drive things forward. It's beyond the scope of this one bug though. I'll try and get things moving on that front, it's something we've needed to do for some time.

@Thierry, have you had a chance to review Angelo's commit for this?
Comment 45 Thierry Vignaud 2014-05-25 20:04:33 CEST
See above
Comment 46 claire robinson 2014-05-26 10:45:18 CEST
I'm not sure if that's a yes or a no Thierry, sorry.

I've read the discussion through and there was a further commit in comment 39 on 17th May which from the changelog (to a non dev) sounds like it would deal with the issue you raised before.
Comment 47 James Kerr 2014-05-31 12:39:22 CEST
So far as I can tell the proposed package correctly resolves this bug.


In order to create a realistic environment for testing, I moved the synergy 
and synergy-gui packages in my local mirror from backports_testing to backports 
and ran genhdlist2 on those directories to update the media info.

I confirmed that the bug still existed by searching for synergy. Since I did 
not have synergy installed no package was found, as expected.

I then installed the proposed rpmdrake-6.10-1.0.mga4.noarch.rpm 

I searched for synergy and only the release version was displayed. I installed 
the two synergy packages and searched again. Still only the, now installed, 
release version was found.

I enabled the backports repo and searched again. The installed release version 
was displayed as was the backports version which was marked as a potential 
update.

Using the first drop down to display available updates, correctly displayed the 
backported synergy packages.

As there were other updates available, I ran Mageia update. It correctly 
ignored the backports packages, since I had not tagged that repo as an update 
source.

The only regression that I observed is that the backports option in the first 
filter no longer displays packages from inactive backports repo's.

That option was added in Mandriva because it was felt that there should be an 
easy way for users to know what backports were available, without enabling the 
backports repo. However, in Mageia we have Mageia AppDB, where available 
backports can be found with a couple of mouse clicks. I believe that we will also have a backports_announce mailing list.
Comment 48 Angelo Naselli 2014-05-31 13:05:21 CEST
> The only regression that I observed is that the backports option in the first 
> filter no longer displays packages from inactive backports repo's.
Well that is correct, if we add only the higher version, we cannot have both, as far as i understood from the previous commit.
A further study can be done to add a different kind of filter maybe.
But i think it's not that wrong, if you disable a media, why should have been used for having the package list? but that is my own opinion of course :)
Comment 49 James Kerr 2014-05-31 14:40:30 CEST
I just reported the facts of what I found. :) 

I agree with you, although not everyone does. This option was discussed at length in Mandriva and IIRC Thierry felt very strongly that it was necessary. I don't know if he has modified his opinion.
Comment 50 Angelo Naselli 2014-05-31 15:21:30 CEST
Well the list should be accessible, so i think it is a matter of menu presentation, i don't think showing things that are not installable is correct. And won't not installable until user does not enable the media ;)
IMVHO filters should work on what is enabled. But i don't want to decide I think the community can say what is better, e.g. users, then we can say why is better if it is a matter of coding or user preferences :)

I think we have a problem yet since in some cases the difference between list of updates and inactive backport is still performed. So i wonder if we had an upate and aslo a backport for a package we could have the same problem as we have now with all packages as filter and backport disabled.
James do you have a test case for that? maybe simulating a backport from cauldron of a package that is in updates with a lower version?
The change is easy but i'd like to have a confirmation for that
Comment 51 Angelo Naselli 2014-05-31 16:02:13 CEST
I'd like to add a point more to argue why the behaviour of showing also hidden (not enabled) backports is very odd (not to say wrong) now into backport list we have also all the various backport_testing (core, nonfree,...) so they would be shown even if we have only backports enabled or if we wanted to know only the official -and tested- backports.
There is not any similar situation for core packages, we cannot know which packages are in testing without enabling testing, am i wrong?
Comment 52 James Kerr 2014-05-31 17:02:35 CEST
(In reply to Angelo Naselli from comment #50)

I'll have a go at testing that - it may be tomorrow before I can report back.
Comment 53 James Kerr 2014-05-31 17:06:10 CEST
(In reply to Angelo Naselli from comment #51)

Yes - it has never really made sense to me to display packages from inactive media but I lost the argument back in Mandriva. :)
Comment 54 Angelo Naselli 2014-05-31 18:33:35 CEST
@james comment #52, thanks for testing. Take your time, we want to fix it after all :)
Comment 55 James Kerr 2014-06-02 11:45:56 CEST
I copied deluge from cauldron to 4/backports and performed equivalent tests to those that I reported in comment #47. So far as I can tell everything worked correctly. I was unable to reproduce the bug.


One anomaly which I failed to observe during my earlier tests is that once the backports repo has been enabled, when it is subsequently de-activated rpmdrake continues to display packages from that repo both as potential updates and in the backports filter, but only when the updates or release package is already installed. In other words it continues to behave as though the backports repo is enabled, but only with respect to installed packages. I went so far as re-booting to try to get rpmdrake to "forget" about the backports, but it still displayed them. :)  This happens with both backports and backports_testing.
Comment 56 Angelo Naselli 2014-06-02 12:54:10 CEST
on Comment #55, James the test i meant was without updating e.g.:
foo 1.0.1-1 is in core
foo 1.0.1-1.1 is in updates
foo 1.0.2 is backports

- case one
foo is not installed.
- case two
foo 1.0.1-1 is installed but not updated
- case three 
foo 1.0.1-1.1 is installed
In all those cases you should filter foo with All, Updates, Backports
and check which version are shown if any.

Concerning the second problem, i cannot see it here, my tests are performed all
by enabling and disabling bp media, because that re-filter all.
But i don't have a package that is both in bp and updates though...
Comment 57 Angelo Naselli 2014-06-02 13:01:24 CEST
@Thierry, @Claire which is the best behaviour, showing either updates and backports, or just the higher version. Because i can understand people who wants to enable bp, but to install only some of them. But I believe it should be related to urpmi cli behaviour, which is in that case? i think it is going to install higher version e.g. the backported one.
Comment 58 James Kerr 2014-06-02 15:24:23 CEST
(In reply to Angelo Naselli from comment #56)
Those are the tests that I ran. I used deluge as the test case (after copying the cauldron package to backports) since there was an  updates package available and I did not initially have it installed. (It also did not have a host of additional dependencies.)

I can only report what I see.  :)
Comment 59 Angelo Naselli 2014-06-02 16:02:33 CEST
Great so it seems not too far from a solution :)
James, could you please back-up
/usr/lib/perl5/vendor_perl/5.18.1/Rpmdrake/pkg.pm and change the line
into subroutine get_pkgs
backports => [ @inactive_backports, @active_backports ],
into 
backports => [ @active_backports ],

And see if you have the wrong side effect you described?
Since we don't show backports if not enabled at this moment there is no
reason to set both active and inactive as backports.
Thanks James, if you have problem in doing that i could append the whole file here.
Comment 60 James Kerr 2014-06-02 19:07:12 CEST
(The line that I changed is 597.)

The anomaly is partially cured. The "Backports" filter list is now empty (when backports repo's are inactive) but packages from inactive backports and backports_testing still show in the "All updates" filter list and are displayed when using the "Find" box to search for packages, if there is an earlier version of the package already installed.
Comment 61 Angelo Naselli 2014-06-02 19:36:44 CEST
could you please add a screenshot, so that i can be sure to have understood correctly :) thanks.
Comment 62 Angelo Naselli 2014-06-02 20:30:14 CEST
Created attachment 5185 [details]
gui.pm to test

James could you please test this gui.pm 
(backup /usr/lib/perl5/vendor_perl/5.18.1/Rpmdrake/gui.pm and use overwrite it with this one)?
Tell me if it's better after.
TIA
Comment 63 James Kerr 2014-06-02 20:42:29 CEST
Created attachment 5186 [details]
Result of using Find box

Here's the first screenshot that you asked for
Comment 64 James Kerr 2014-06-02 20:45:01 CEST
Created attachment 5187 [details]
Results from updates filter

And here's the second
Comment 65 James Kerr 2014-06-02 20:47:36 CEST
For greater certainty:

$ urpmq --list-media active
chrome_x86_64
google-earth
Core Release (private1)
Core Updates (private3)
Nonfree Release (private11)
Nonfree Updates (private13)
Tainted Release (private21)
Tainted Updates (private23)
Core 32bit Release (private31)
Core 32bit Updates (private32)
Nonfree 32bit Release (private36)
Nonfree 32bit Updates (private37)
Tainted 32bit Release (private41)
Tainted 32bit Updates (private42)
Comment 66 James Kerr 2014-06-02 20:51:30 CEST
The new version of gui.pm does not seem to have changed anything. The anomaly is still present.
Comment 67 Angelo Naselli 2014-06-02 21:39:38 CEST
Sorry for that it was a try, i have to build a mirror in some way to test and understand and debug the problem, so I need more time.
Anyway, just to be sure, did you test with both gui and pkg changed?
Comment 68 James Kerr 2014-06-02 22:26:25 CEST
I have not reverted the change to pgk.pm and so my most recent tests were with the modified pkg.pm and the new gui.pm.
Comment 69 Mageia Robot 2014-06-27 00:38:03 CEST
commit dc9950451a6c83815e2f680f9eaba6da31a18300
Author: Thierry Vignaud <thierry.vignaud@...>
Date:   Thu Jun 26 23:07:21 2014 +0200

    Revert "(get_pkgs) only display latest updates, not all of them (mga#2258, mga#4534)"
    
    This reverts commit d600679cdaf402a0f97bbbd76e7fa04c06c5a37e.
    
    Rationale: it wreaks havoc (mga#12766)
    
    Conflicts:
    	Rpmdrake/pkg.pm
---
 Commit Link:
   http://gitweb.mageia.org/software/rpmdrake/commit/?id=dc9950451a6c83815e2f680f9eaba6da31a18300

 Bug links:
   Mageia
      https://bugs.mageia.org/show_bug.cgi?id=4534
      https://bugs.mageia.org/show_bug.cgi?id=12766
      https://bugs.mageia.org/show_bug.cgi?id=2258
Comment 70 Manuel Hiebel 2014-06-27 01:55:46 CEST
(In reply to Mageia Robot from comment #69)
> commit dc9950451a6c83815e2f680f9eaba6da31a18300
> Author: Thierry Vignaud <thierry.vignaud@...>
> Date:   Thu Jun 26 23:07:21 2014 +0200
> 
>     Revert "(get_pkgs) only display latest updates, not all of them
> (mga#2258, mga#4534)"
>     
>     This reverts commit d600679cdaf402a0f97bbbd76e7fa04c06c5a37e.
>     
>     Rationale: it wreaks havoc (mga#12766)
>     
>     Conflicts:
>     	Rpmdrake/pkg.pm
> ---
>  Commit Link:
>   
> http://gitweb.mageia.org/software/rpmdrake/commit/
> ?id=dc9950451a6c83815e2f680f9eaba6da31a18300
> 
>  Bug links:
>    Mageia
>       https://bugs.mageia.org/show_bug.cgi?id=4534
>       https://bugs.mageia.org/show_bug.cgi?id=12766
>       https://bugs.mageia.org/show_bug.cgi?id=2258

This will bring back old view with everything listed several time no ? 
Ihmo, we should not break 'updates usability', to provide backport
Comment 71 Thierry Vignaud 2014-06-27 09:21:47 CEST
Created attachment 5216 [details]
keep only latest package, but per medium

Well that was a very recent change anyway.

Can you test this patch instead?
against older rpmdrake
Comment 72 Angelo Naselli 2014-06-27 10:11:29 CEST
Sorry Thierry i don't want to bother you and i'm really happy if you can fix this
issue, but didn't we agree that 
 	$l{$key} = $pkg if !$l{$key} || $l{$key}->compare($pkg);
was wrong and we should use compare_pkg instead?
Comment 73 Thierry Vignaud 2014-06-27 10:28:45 CEST
As we said in french, it was to see if people were listening :-)
Indeed I'd forgotten... :-(
Comment 74 Thierry Vignaud 2014-06-27 10:31:10 CEST
Created attachment 5217 [details]
keep only latest package, but per medium (v2)

updated patch per Angelo Naselli's reminder
Comment 75 Angelo Naselli 2014-06-27 11:14:30 CEST
for some reason your patch on the original file failed. Anyway i applied it.
Things improved, now we can see both package always.
But if i don't enable backports i can install them anyway, since i can see them
in backports, but what is worst is that i can do that also if they are in testing,
so update->testing behaves different.
Then if i enable backports, i can see the backport package only in Backports and not if i filter as ALL packages again different behaviour compared Updates.

I think this patch could be released if someone experienced is working as i expected. But it's not a solution because *at least* backports-testing should work differently.
Comment 76 Thierry Vignaud 2014-06-27 12:25:17 CEST
That's the purpose of backports, they're not enabled (so that auto-select won't wrongly take packages from them) but we offer to pick packages from them, 
Think cherry-picking

As for */testing, AFAIC that's a (separate) media.cfg issue where backport * testing are not tagged as backport
Comment 77 Angelo Naselli 2014-06-27 13:04:45 CEST
Well if so let's say, cherry-picking backports works, enabled backports no.

As far as I can say bp-testing is tagged as backport, indeed that is the only one
repo available as backport atm, just filter as backports with your patch and
see... (despite of it is enabled or not).

quoted dkms-bbswitch:
Version: 0.8-1.mga4
        Group: System/Kernel and hardware
        Architecture: noarch
        Size: 24 KB
        Medium: Core 32bit Backports Testing

As said, we can unlock the bp repos if this patch works also for other testers,
but
1) All filter does not work for those bp repos that are enabled
   in this case IMO the best solution should be to see upper pkg per enabled repo
   if not installed, or also any other upper packages if installed, update icon   
   shown just for updates

2) Backports filter should show only the backport repo and not testing, as cherry-
   picking, in addition to any other enabled bp repos.
Comment 78 Mageia Robot 2014-06-27 19:10:05 CEST
commit d082f871e95f3977dd3198090ed21e44a2a2ae0e
Author: Thierry Vignaud <thierry.vignaud@...>
Date:   Fri Jun 27 18:49:01 2014 +0200

    keep only latest package per medium, not globally (mga#12766)
---
 Commit Link:
   http://gitweb.mageia.org/software/rpmdrake/commit/?id=d082f871e95f3977dd3198090ed21e44a2a2ae0e
Comment 79 Mageia Robot 2014-06-27 19:11:59 CEST
commit 9a645910e2160677b5e919fb54d67027e9d9e67e
Author: Thierry Vignaud <thierry.vignaud@...>
Date:   Fri Jun 27 18:49:01 2014 +0200

    keep only latest package per medium, not globally (mga#12766)
    
    Conflicts:
    	NEWS
---
 Commit Link:
   http://gitweb.mageia.org/software/rpmdrake/commit/?id=9a645910e2160677b5e919fb54d67027e9d9e67e
Comment 80 Thomas Backlund 2014-07-04 21:06:37 CEST
Update pushed:
http://advisories.mageia.org/MGAA-2014-0144.html

Note You need to log in before you can comment on or make changes to this bug.