plugwash / raspbian-tools

10 stars 3 forks source link

IndexError: list index out of range when using sourcepool argument #2

Closed herrbischoff closed 4 years ago

herrbischoff commented 4 years ago

Environment

# uname -a
FreeBSD vandusen.port0.org 12.1-RELEASE-p6 FreeBSD 12.1-RELEASE-p6 GENERIC  amd64

# python3 -V
Python 3.7.7

Issue

When trying to create a new mirror from scratch using

cd /pub/mirror/www/raspbian
/home/mirror/bin/raspbmirror.py --tmpdir /pub/mirror/tmp --sourcepool /pub/mirror/www/debian/pool

source and package lists are downloaded correctly. However, when file download starts, the script immediately aborts and throws an error:

Traceback (most recent call last):
  File "/home/mirror/bin/raspbmirror.py", line 564, in <module>
    getfile(filepath,sha256,size)
  File "/home/mirror/bin/raspbmirror.py", line 219, in getfile
    if pathsplit[1] == b'pool':
IndexError: list index out of range

I have tried to debug this issue (the above line numbers are slightly different since I inserted several print statements) but am stumped by the relevant line

https://github.com/plugwash/raspbian-tools/blob/b347112e4d33e5cd0aebbfc347dda20e67339d54/raspbmirror.py#L218

This references

https://github.com/plugwash/raspbian-tools/blob/b347112e4d33e5cd0aebbfc347dda20e67339d54/raspbmirror.py#L204

which at the time of execution contains

[b'favicon.ico']

This effectively prevents using the --sourcepool argument for me.

plugwash commented 4 years ago

I think the if statement needs to be changed to

if (len(pathsplit) > 1) and (pathsplit[1] == b'pool'):

plugwash commented 4 years ago

To explain further the issue is it's trying to check if the second element of the path is "pool" (and if-so go into some special case code related to the sourcepool option) but sometimes a path only has one element.

I suspect the reason this bug happened is that when I tested the new feature I did so on an update to an existing mirror rather than a from-scratch run and/or I did the test with something other than the main raspbian repo.

herrbischoff commented 4 years ago

Thanks for explaining it. I have now changed the respective line it appears to work as expected.

For your convenience I have gone ahead an created a pull request containing this change.

herrbischoff commented 4 years ago

Just a heads up: now it runs through completely. However, stage 4 always ends with

file name contains unexpected characters
plugwash commented 4 years ago

Thanks for the report, I have just pushed a commit to improve that error message. Can you try it again?

herrbischoff commented 4 years ago

I have just downloaded the newest script and started a run. Will post the results here.

herrbischoff commented 4 years ago

The run finished, and here's the last couple of lines from the output:

[...]
removing raspbian/pool/main/p/pd-iemmatrix/pd-iemmatrix_0.3.2-1+b5_armhf.deb
removing raspbian/pool/main/s/supercollider/supercollider-supernova_3.10.0+repack-1+rpi2_armhf.deb
removing raspbian/pool/main/i/ivtools/ivtools-bin_1.2.11a4-1_armhf.deb
removing raspbian/pool/main/k/kamailio/kamailio-websocket-modules_5.3.4-1_armhf.deb
file name None contains unexpected characters
plugwash commented 4 years ago

Apologies, I screwed up in my error reporting improvements, can you update and try again.

herrbischoff commented 4 years ago

Well, here you go:

[...]
removing raspbian/pool/main/libv/libvncserver/libvncserver_0.9.9+dfsg2-6.1+deb8u7.debian.tar.xz
removing raspbian/pool/main/libv/libvncserver/libvncserver0_0.9.9+dfsg2-6.1+deb8u7_armhf.deb
Traceback (most recent call last):
  File "/home/mirror/bin/raspbmirror.py", line 726, in <module>
    ensuresafepath(filepath)
  File "/home/mirror/bin/raspbmirror.py", line 99, in ensuresafepath
    print("component "+ascii(component)+" in path "+path+" contains unexpected characters")
TypeError: can only concatenate str (not "bytes") to str

As far as I understand Python, it should probably be str(component), as ascii(component) can also byte-encode.

plugwash commented 4 years ago

"ascii" produces a string, it's similar to repr but with non-ascii characters escaped. The issue is I forgot to put any kind of conversion on the variable "path", I have now pushed a fix for that.

Apologies for the number of round trips this is taking, but since I don't currently know how to provoke the error I can't easily test the error handling code-path.

herrbischoff commented 4 years ago

I see. Will test again now. However, every run takes a lot more time than the old rsync method did. Is there any way to improve performance?

plugwash commented 4 years ago

It should perform a lot better once it's working properly, right now it's likely doing a bunch of re-checking because a run is never completing properly.

herrbischoff commented 4 years ago

Here we go:

[...]
removing raspbian/pool/main/i/imagemagick/libmagick++-6.q16hdri-dev_6.9.10.23+dfsg-2.1_armhf.deb
removing raspbian/pool/main/f/feedbackd/feedbackd_0.0.0+git20200420.orig.tar.gz
removing raspbian/pool/main/m/modemmanager/libmm-glib0_1.12.10-0.2_armhf.deb
removing raspbian/pool/main/g/golang-github-kardianos-service/golang-github-kardianos-service-dev_1.0.0-1_all.deb
component b'' in path b'/pub/mirror/tmp/raspbian~pool~main~g~gigalomania~gigalomania-data_1.0+ds1-1_all.deb.new' contains unexpected characters
plugwash commented 4 years ago

Thanks, looks like removals from the temporary directory (which should normally be a rare occurrence, my best guess is that this one was caused by running without the sourcepool argument, then ctrl-cing and then running with the sourcepool argument) were broken.

I have pushed a commit that should fix them.

herrbischoff commented 4 years ago

Thanks, I have started another run with the updated script.

herrbischoff commented 4 years ago

The last fix indeed did the trick. The update went through with no error code or error message.

herrbischoff commented 4 years ago

Just a quick feedback that the script runs stable for 10 days now. Thanks again for the help!