diff mbox

[OpenWrt-Devel,PATCHv2] scripts/feeds: observe -p flag for preferential feeds

Message ID 1432742616-1328-1-git-send-email-karlp@tweak.net.au
State Accepted
Headers show

Commit Message

Karl Palsson May 27, 2015, 4:03 p.m. UTC 
From: Karl Palsson <karlp@remake.is>

lookup_target was trampling the $feed variable, resulting in the -p flag
no longer preferentially installing from the named feed.

Make sure to use a local variable for this instead.

Signed-off-by: Karl Palsson <karlp@remake.is>
---
Changes since v1:
 Make sure to not just trample a global variable.  Aren't globals fun. Thanks
 to Felix for pointing it out

 scripts/feeds | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

John Crispin May 31, 2015, 5:39 p.m. UTC | #1 
On 27/05/2015 18:03, Karl Palsson wrote:
> +	my $this_feed_target = lookup_target($feed, $name);
> +	$this_feed_target and do {

how about just calling it $target ?
Karl Palsson June 1, 2015, 12:50 p.m. UTC | #2 
John Crispin <blogic@openwrt.org> wrote:
> On 27/05/2015 18:03, Karl Palsson wrote:
> > +	my $this_feed_target = lookup_target($feed, $name);
> > +	$this_feed_target and do {
> 
> how about just calling it $target ?

Because even though the method is "lookup_target" it actually returns a
feed for the target $name.

I could change the method to lookup_feed_for_target too, if that would
help.

Sincerely,
Karl Palsson
Karl Palsson June 9, 2015, 2:16 p.m. UTC | #3 
Karl Palsson <karlp@tweak.net.au> wrote:
> 
> John Crispin <blogic@openwrt.org> wrote:
> > On 27/05/2015 18:03, Karl Palsson wrote:
> > > +	my $this_feed_target = lookup_target($feed, $name);
> > > +	$this_feed_target and do {
> > 
> > how about just calling it $target ?
> 
> Because even though the method is "lookup_target" it actually returns a
> feed for the target $name.
> 
> I could change the method to lookup_feed_for_target too, if that would
> help.

Hi,

Is anything else needed here?  Would be nice to get this fixed before
it's branched.

Sincerely,
Karl Palsson
Karl Palsson June 23, 2015, 9:51 a.m. UTC | #4 
Karl Palsson <karlp@tweak.net.au> wrote:
> > John Crispin <blogic@openwrt.org> wrote:
> > > On 27/05/2015 18:03, Karl Palsson wrote:
> > > > +	my $this_feed_target = lookup_target($feed, $name);
> > > > +	$this_feed_target and do {
> > > 
> > > how about just calling it $target ?
> > 
> > Because even though the method is "lookup_target" it actually returns a
> > feed for the target $name.
> > 
> > I could change the method to lookup_feed_for_target too, if that would
> > help.
> 
> Is anything else needed here?  Would be nice to get this fixed before
> it's branched.
> 

Anything?  It now should be backported to CC as well, given that this is
documented behaviour of scripts/feeds that's regressed.

Sincerely,
Karl Palsson
diff mbox

Patch

diff --git a/scripts/feeds b/scripts/feeds
index a6be9cc..55756d9 100755
--- a/scripts/feeds
+++ b/scripts/feeds
@@ -450,10 +450,10 @@  sub install_package {
 	my $force = shift;
 	my $ret = 0;
 
-	$feed = lookup_target($feed, $name);
-	$feed and do {
+	my $this_feed_target = lookup_target($feed, $name);
+	$this_feed_target and do {
 		$installed_targets{$name} and return 0;
-		install_target($feed, $name);
+		install_target($this_feed_target, $name);
 		return 0;
 	};