| Summary: | Installer no longer respects default rpmsrate level when selecting packages | ||
|---|---|---|---|
| Product: | Mageia | Reporter: | Martin Whitaker <mageia> |
| Component: | Installer | Assignee: | Mageia tools maintainers <mageiatools> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | Normal | CC: | jani.valimaa, thierry.vignaud |
| Version: | Cauldron | Keywords: | PATCH |
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Source RPM: | CVE: | ||
| Status comment: | |||
| Attachments: |
old install2 test case
current install2 test case new Getopt option install2 test case old install2 test case current install2 test case new Getopt option install2 test case comparing files generated by test-old & test.pl comparing files generated by test-old & test-new.pl prevent autovivification (mga#20551) prevent autovivification (mga#20551) ignore case for options prevent autovivification (mga#20551) |
||
Thx for spotting this. On the other hand, other variables might be affected. What would be the Getopt::Long option? Keywords:
(none) =>
PATCH (In reply to Thierry Vignaud from comment #1) > What would be the Getopt::Long option? http://perldoc.perl.org/Getopt/Long.html#Storing-options-values-in-a-hash Created attachment 9135 [details]
old install2 test case
Created attachment 9136 [details]
current install2 test case
Created attachment 9137 [details]
new Getopt option install2 test case
Created attachment 9138 [details]
old install2 test case
v2: actually use args
Attachment 9135 is obsolete:
0 =>
1 Created attachment 9139 [details]
current install2 test case
v2: actually use args
Attachment 9136 is obsolete:
0 =>
1 Created attachment 9140 [details]
new Getopt option install2 test case
v2: actually use args
Attachment 9137 is obsolete:
0 =>
1 Created attachment 9141 [details]
comparing files generated by test-old & test.pl
Indeed we can see a lot of autovivification, using:
perl test-old.pl --flang fr
perl test.pl --flang=fr
diff -u o_old.pm o.pm >old_and_current2.diff
Note that with the old code that didn't used Getopt, "LANG=fr_FR.UTF-8" from /proc/cmdline overrides --flang...
Created attachment 9142 [details]
comparing files generated by test-old & test-new.pl
There's quite a lot less of autovivification, using:
perl test-old.pl --flang fr
perl test-new.pl --flang=fr
diff -u o_old.pm o_new.pm >old_and_new_option_result2.diff
I think we can safely uses the Getopt option as we can play with several drakx options using those scripts, in order to make sure there's no unwanted side effects eg:
for i in test-old.pl test-new.pl;do perl ./$i --flang fr --keyboard ru --debug_urpmi --deploops --justdb --text --fdisk --nodmraid --compsslistlevel=4 --useless_thing_accepted ;done; diff -u o_*pm>l
Which nicely that I forgot to do handle the case for the level in test-new.pl:
'compsslistlevel=s' => \$o->{compssListLevel},
So I think we can safely do the change.
Created attachment 9143 [details]
prevent autovivification (mga#20551)
Just moving parse_args() from test-new.pl back into install::install2
Created attachment 9144 [details]
prevent autovivification (mga#20551)
also prevent autovivification of 'security'
Attachment 9143 is obsolete:
0 =>
1 Created attachment 9145 [details]
ignore case for options
Created attachment 9146 [details]
prevent autovivification (mga#20551)
v3: actually handles 'compssListLevel' !!!
Attachment 9144 is obsolete:
0 =>
1 Looks fairly safe to me. We may have a few more bugs like 20521/20522 to fix up, but I doubt there'll be that many. commit 73743912c3d2eb61012a9f4273972e3701dc6b55
Author: Thierry Vignaud <thierry.vignaud@...>
Date: Thu Mar 23 16:17:28 2017 +0100
prevent autovivification (mga#20551)
commit 560f9ca983d743cc42701c24546ac8c2080a13fe introduced a regression
in that a lot of variables got autovivificated
This results in a bug for some of them, at least for 'compssListLevel'
The solution is to store options values in a hash (which actually
simplifies here):
http://perldoc.perl.org/Getopt/Long.html#Storing-options-values-in-a-hash
Thanks Martin Whitaker for the suggestion
---
Commit Link:
http://gitweb.mageia.org/software/drakx/commit/?id=73743912c3d2eb61012a9f4273972e3701dc6b55
Fixed Status:
ASSIGNED =>
RESOLVED |
This bug was introduced in commit 560f9ca98 ("simplify by switching to Getopt::Long"). If the compsslistlevel option is not present on the command line, Getopt::Long still adds a compssListLevel key to the $o hash, setting it to an empty string. This then prevents $o->{compssListLevel} being set to the default rpmsrate level in steps_interactive::choosePackages() (because add2hash_ doesn't replace an existing key). Reading the documentation for Getopt::Long, there is a way to prevent it adding to hashes if an option isn't present, but that might be a bit risky at this stage in mga6 (might introduce other bugs). So maybe something like --- a/perl-install/install/install2.pm +++ b/perl-install/install/install2.pm @@ -580,7 +580,7 @@ sub parse_args { 'suppl=s' => \$o->{supplmedia}, askmedia => \$o->{askmedia}, restore => \$::isRestore, - 'compsslistlevel=s' => \$o->{compssListLevel}, + 'compsslistlevel=s' => sub { $o->{compssListLevel} = $_[1] if $_[1] > 0 }, ); ($cfg, $patch); (not tested).