Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#17842 closed defect (fixed)

scripts/feeds update -a can fail rather silently for git feeds

Reported by: hnyman Owned by: developers
Priority: normal Milestone: Chaos Calmer 15.05
Component: toolchain Version: Trunk
Keywords: feeds git Cc:

Description

"scripts/feeds update -a" can fail rather silently for feeds using git, as the script does not pause when updating a feed fails. Instead it prints the error message and calmly continues to the next feed. It is very easy to overlook update errors with the feeds updated first, as their text scrolls rapidly away from the screen.

This has not been a big problem with svn feeds, as svn update stops with a conflict message and interactively forces the user to resolve or postpone the conflict. In any case the error will be noticed by the user.

Majority of the feeds use now git, so this silent failure can affect users doing private builds in an increasing amount.

Toolchain script should be improved to better inform user about failures. Preferably updating should stop after a failure.

Below is an example of update failing and script continuing:

perus@v1404:/Openwrt/barrier$ ./scripts/feeds update -a
Updating feed 'packages' from 'https://github.com/openwrt/packages.git;for-14.07' ...
remote: Counting objects: 17, done.
remote: Compressing objects: 100% (15/15), done.
remote: Total 17 (delta 10), reused 8 (delta 1)
Unpacking objects: 100% (17/17), done.
From https://github.com/openwrt/packages
   62031da..dc26009  for-14.07  -> origin/for-14.07
Updating 62031da..dc26009
error: Your local changes to the following files would be overwritten by merge:
	utils/collectd/Makefile
Please, commit your changes or stash them before you can merge.
Aborting
failed.
Updating feed 'luci' from 'http://git.openwrt.org/project/luci.git;luci-0.12' ...
Already up-to-date.
Create index file './feeds/luci.index' 
Updating feed 'routing' from 'https://github.com/openwrt-routing/packages.git;for-14.07' ...
...

The script prints "failed.", but does not break the updating process. It looks like the "update_feed" function returns an error code 1, but that is not checked in "update" which continues to the next feed.
Return 1 as error:
https://dev.openwrt.org/browser/trunk/scripts/feeds#L547
Call to update_feed without any error monitoring:
https://dev.openwrt.org/browser/trunk/scripts/feeds#L585

Attachments (0)

Change History (7)

comment:1 Changed 3 years ago by hnyman

I am not a perl expert, but this seems to work for me. The script stops updating after failing to update a feed.

(The script continues to the refresh_config step despite a possible failure in updating, so the stopping action just prevents the other feeds from updating.)

--- trunk/scripts/feeds
+++ trunk/scripts/feeds
@@ -585,7 +585,8 @@
 	if ( ($#ARGV == -1) or $opts{a}) {
 		foreach my $feed (@feeds) {
 			my ($type, $name, $src) = @$feed;
-			update_feed($type, $name, $src, $perform_update);
+			next unless update_feed($type, $name, $src, $perform_update) == 1;
+			last;
 		}
 	} else {
 		while ($feed_name = shift @ARGV) {

comment:2 Changed 3 years ago by hnyman

This bug could be closed.
Fixed by r42891

comment:3 Changed 3 years ago by nbd

  • Resolution set to fixed
  • Status changed from new to closed

comment:4 Changed 3 years ago by anonymous

We're calling feeds update from a shell script, but even in the case of failures described above, the script returns 0 instead of 1.

It would help if the script would propagate the error all the way up so it can be caught by the caller.

comment:5 Changed 3 years ago by hnyman

I think that replacing "last" with "exit(1)" should work for you. Config will not be refreshed, but that is not a problem as you want to break compiling in any case.

I am not sure if that if good enough to be implemented in the official sources, but that should help you.

comment:6 Changed 3 years ago by anonymous

I can of course hack this locally, but since we keep building (and updating) from OpenWRT trunk it would be nice to get this fixed in upstream, unless there's a compelling reason for this script not to behave this way.

comment:7 Changed 3 years ago by anonymous

Another issue (not sure if this is addressed anywhere), is that merge conflicts of git pull are often causing this issue. The "update" calls git pull, it would be nice if it would have an alternative to use fetch & reset --hard which would ensure any upstream conflicts could be automatically overwritten.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.