TOOLS/binary_codecs.sh update
hi, The original author of TOOLS/binary_codecs.sh proposed three changes, two of which I found clear and easy to understand and committed it. The last one made me think. I'm talking about this change: Index: binary_codecs.sh =================================================================== --- binary_codecs.sh (Revision 32485) +++ binary_codecs.sh (Arbeitskopie) @@ -20,6 +20,36 @@ cd $CODECDIR [ -d mplayer_binary_codecs ] || mkdir -v mplayer_binary_codecs +mywget () +{ + # note the the URL must be the first option + t=`tempfile` + #check that we are not redirected + if LANG=C LC_ALL=C LC_MESSAGES=C wget --spider -S -o $t "$1" ; then + if egrep -qx ' *HTTP.* 302 Found' $t ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $t | head -n 1 + echo See more info in the log in $t + echo + return 1 + else + rm $t + fi + else + echo Errors while checking URL "$1" + echo See the log in $t + return 1 + fi + url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi +} choosemirror () { @@ -27,7 +57,7 @@ #if [ ! -r mirrors ] || find mirrors -mtime +20 ; then echo "Downloading mirrors list" - wget -nv -c -N $MYSITE/mirrors || true + mywget $MYSITE/mirrors -nv -c -N || true #fi if [ ! -r bestsites ] || [ mirrors -nt bestsites ] || \ find bestsites -mtime +20 | grep -q bestsites ; then @@ -65,15 +95,13 @@ test -r $list || list=$PREFDIR/mirrors cat $list | while read mainsite ; do echo Downloading $filename from $mainsite ... - wget -c -N $mainsite/$dir/$filename || true - if [ -r "$filename" ] ; then + if mywget $mainsite/$dir/$filename -c -N && [ -r "$filename" ] ; then UNPACK "$filename" return 0 fi done else - wget -c -N $url/$dir/$filename || true - if [ -r "$filename" ] ; then + if mywget $url/$dir/$filename -c -N && [ -r "$filename" ] ; then UNPACK "$filename" return 0 fi @@ -159,13 +187,13 @@ cd $PREFDIR #if [ ! -r codecs_list ] || find codecs_list -mtime +20 ; then echo "Getting codecs list" - wget -nv -c -N $MYSITE/codecs_list || true + mywget $MYSITE/codecs_list -nv -c -N || true #fi cd $PREFDIR echo Downloading MD5 sums from main site [ -r MD5SUMS ] && mv MD5SUMS MD5SUMS.bak - if wget -nv -N http://www.mplayerhq.hu/MPlayer/releases/codecs/MD5SUMS ; then + if mywget http://www.mplayerhq.hu/MPlayer/releases/codecs/MD5SUMS -nv -N ; then [ -r MD5SUMS.bak ] && rm MD5SUMS.bak else echo "failed" My thoughts: - I haven't tested it myself yet, but if it works (I suppose it does), I'm inclined to include it - is that mywget approach somewhat sane? - should we host the files at http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu instead? I mean we are in a good position to keep them up-to-date. OTOH, this is a debian/ubuntu specific service, so before doing this, I'd like to hear some comments on this. -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4
On Wed, Oct 13, 2010 at 05:53:58PM +0200, Reinhard Tartler wrote:
The original author of TOOLS/binary_codecs.sh proposed three changes, two of which I found clear and easy to understand and committed it. The last one made me think. I'm talking about this change:
--- binary_codecs.sh (Revision 32485) +++ binary_codecs.sh (Arbeitskopie) @@ -20,6 +20,36 @@
+mywget () +{ + # note the the URL must be the first option + t=`tempfile`
horrible variable name $() is preferable to ``.
+ #check that we are not redirected + if LANG=C LC_ALL=C LC_MESSAGES=C wget --spider -S -o $t "$1" ; then
These variables should be set globally I think.
+ if egrep -qx ' *HTTP.* 302 Found' $t ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $t | head -n 1 + echo See more info in the log in $t + echo + return 1 + else + rm $t + fi + else + echo Errors while checking URL "$1" + echo See the log in $t + return 1 + fi
Indentation looks pretty random.
+ url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi
random indeed
My thoughts:
- I haven't tested it myself yet, but if it works (I suppose it does), I'm inclined to include it - is that mywget approach somewhat sane?
I haven't given it enough thought to form an opinion yet.
- should we host the files at http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu instead?
Yes. Diego
Thanks for the thorough review! On Do, Okt 28, 2010 at 09:50:28 (CEST), Diego Biurrun wrote:
On Wed, Oct 13, 2010 at 05:53:58PM +0200, Reinhard Tartler wrote:
The original author of TOOLS/binary_codecs.sh proposed three changes, two of which I found clear and easy to understand and committed it. The last one made me think. I'm talking about this change:
--- binary_codecs.sh (Revision 32485) +++ binary_codecs.sh (Arbeitskopie) @@ -20,6 +20,36 @@
+mywget () +{ + # note the the URL must be the first option + t=`tempfile`
horrible variable name
$() is preferable to ``.
changed
+ #check that we are not redirected + if LANG=C LC_ALL=C LC_MESSAGES=C wget --spider -S -o $t "$1" ; then
These variables should be set globally I think.
changed
+ if egrep -qx ' *HTTP.* 302 Found' $t ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $t | head -n 1 + echo See more info in the log in $t + echo + return 1 + else + rm $t + fi + else + echo Errors while checking URL "$1" + echo See the log in $t + return 1 + fi
Indentation looks pretty random.
fixed. I took the freedom to normalize the file before
+ url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi
random indeed
fixed
My thoughts:
- I haven't tested it myself yet, but if it works (I suppose it does), I'm inclined to include it - is that mywget approach somewhat sane?
I haven't given it enough thought to form an opinion yet.
- should we host the files at http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu instead?
Yes.
Can you either add me to group www-data, or otherwise arrange me write access so that I can install these files? Or shall I rather put it in my public_html in my home? Index: binary_codecs.sh =================================================================== --- binary_codecs.sh (revision 32561) +++ binary_codecs.sh (working copy) @@ -12,6 +12,10 @@ CODECDIR=/usr/lib/codecs PREFDIR=/var/lib/mplayer/prefs MYSITE='http://people.debian.org/~mennucc1/mplayer' +LANG=C +LC_ALL=C +LC_MESSAGES=C +export LANG LC_ALL LC_MESSAGES dpkgarch=$(dpkg --print-architecture) @@ -20,13 +24,44 @@ cd $CODECDIR [ -d mplayer_binary_codecs ] || mkdir -v mplayer_binary_codecs +mywget () +{ + # note the the URL must be the first option + tempf=$(tempfile) + #check that we are not redirected + if wget --spider -S -o $tempf "$1" ; then + if egrep -qx ' *HTTP.* 302 Found' $tempf ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $tempf | head -n 1 + echo See more info in the log in $tempf + echo + return 1 + else + rm $tempf + fi + else + echo Errors while checking URL "$1" + echo See the log in $tempf + return 1 + fi + url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi +} + choosemirror () { cd $PREFDIR #if [ ! -r mirrors ] || find mirrors -mtime +20 ; then echo "Downloading mirrors list" - wget -nv -c -N $MYSITE/mirrors || true + mywget $MYSITE/mirrors -nv -c -N || true #fi if [ ! -r bestsites ] || [ mirrors -nt bestsites ] || \ find bestsites -mtime +20 | grep -q bestsites ; then @@ -62,15 +97,13 @@ test -r $list || list=$PREFDIR/mirrors cat $list | while read mainsite ; do echo Downloading $filename from $mainsite ... - wget -c -N $mainsite/$dir/$filename || true - if [ -r "$filename" ] ; then + if mywget $mainsite/$dir/$filename -c -N && [ -r "$filename" ] ; then UNPACK "$filename" return 0 fi done else - wget -c -N $url/$dir/$filename || true - if [ -r "$filename" ] ; then + if mywget $url/$dir/$filename -c -N && [ -r "$filename" ] ; then UNPACK "$filename" return 0 fi @@ -154,13 +187,13 @@ cd $PREFDIR #if [ ! -r codecs_list ] || find codecs_list -mtime +20 ; then echo "Getting codecs list" - wget -nv -c -N $MYSITE/codecs_list || true + mywget $MYSITE/codecs_list -nv -c -N || true #fi cd $PREFDIR echo Downloading MD5 sums from main site [ -r MD5SUMS ] && mv MD5SUMS MD5SUMS.bak - if wget -nv -N http://www.mplayerhq.hu/MPlayer/releases/codecs/MD5SUMS ; then + if mywget http://www.mplayerhq.hu/MPlayer/releases/codecs/MD5SUMS -nv -N ; then [ -r MD5SUMS.bak ] && rm MD5SUMS.bak else echo "failed" -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4
On Thu, Oct 28, 2010 at 10:21:22AM +0200, Reinhard Tartler wrote:
Thanks for the thorough review!
Uh, I wouldn't consider it that thorough :)
On Do, Okt 28, 2010 at 09:50:28 (CEST), Diego Biurrun wrote:
On Wed, Oct 13, 2010 at 05:53:58PM +0200, Reinhard Tartler wrote:
The original author of TOOLS/binary_codecs.sh proposed three changes, two of which I found clear and easy to understand and committed it. The last one made me think. I'm talking about this change:
My thoughts:
- I haven't tested it myself yet, but if it works (I suppose it does), I'm inclined to include it - is that mywget approach somewhat sane?
I haven't given it enough thought to form an opinion yet.
- should we host the files at http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu instead?
Yes.
Can you either add me to group www-data, or otherwise arrange me write access so that I can install these files? Or shall I rather put it in my public_html in my home?
Why not add them to the top directory of the homepage repository? BTW, the list of mirrors is outdated.
--- binary_codecs.sh (revision 32561) +++ binary_codecs.sh (working copy) @@ -12,6 +12,10 @@ CODECDIR=/usr/lib/codecs PREFDIR=/var/lib/mplayer/prefs MYSITE='http://people.debian.org/~mennucc1/mplayer' +LANG=C +LC_ALL=C +LC_MESSAGES=C +export LANG LC_ALL LC_MESSAGES
These should be at the very top of the file. I think LC_MESSAGES is redundant and you can directly export the vars.
@@ -20,13 +24,44 @@
+mywget () +{ + # note the the URL must be the first option + tempf=$(tempfile) + #check that we are not redirected + if wget --spider -S -o $tempf "$1" ; then + if egrep -qx ' *HTTP.* 302 Found' $tempf ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $tempf | head -n 1 + echo See more info in the log in $tempf + echo + return 1 + else + rm $tempf + fi + else + echo Errors while checking URL "$1" + echo See the log in $tempf + return 1 + fi + url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi +}
I'm still not sure if this is worth the complexity. Did you guys experience problems with 302 return codes? Diego
On Wed, Nov 03, 2010 at 12:25:23 (CET), Diego Biurrun wrote:
On Thu, Oct 28, 2010 at 10:21:22AM +0200, Reinhard Tartler wrote:
Thanks for the thorough review!
Uh, I wouldn't consider it that thorough :)
On Do, Okt 28, 2010 at 09:50:28 (CEST), Diego Biurrun wrote:
On Wed, Oct 13, 2010 at 05:53:58PM +0200, Reinhard Tartler wrote:
The original author of TOOLS/binary_codecs.sh proposed three changes, two of which I found clear and easy to understand and committed it. The last one made me think. I'm talking about this change:
My thoughts:
- I haven't tested it myself yet, but if it works (I suppose it does), I'm inclined to include it - is that mywget approach somewhat sane?
I haven't given it enough thought to form an opinion yet.
- should we host the files at http://people.debian.org/~mennucc1/mplayer/ somewhere on mplayerhq.hu instead?
Yes.
Can you either add me to group www-data, or otherwise arrange me write access so that I can install these files? Or shall I rather put it in my public_html in my home?
Why not add them to the top directory of the homepage repository? BTW, the list of mirrors is outdated.
--- binary_codecs.sh (revision 32561) +++ binary_codecs.sh (working copy) @@ -12,6 +12,10 @@ CODECDIR=/usr/lib/codecs PREFDIR=/var/lib/mplayer/prefs MYSITE='http://people.debian.org/~mennucc1/mplayer' +LANG=C +LC_ALL=C +LC_MESSAGES=C +export LANG LC_ALL LC_MESSAGES
These should be at the very top of the file.
I think LC_MESSAGES is redundant and you can directly export the vars.
as in: export LANG=C ? isn't that a bashism?
@@ -20,13 +24,44 @@
+mywget () +{ + # note the the URL must be the first option + tempf=$(tempfile) + #check that we are not redirected + if wget --spider -S -o $tempf "$1" ; then + if egrep -qx ' *HTTP.* 302 Found' $tempf ; then + echo Error, there is an unexpected redirection, for the + echo URL "$1" + echo that is redirected to + grep Location $tempf | head -n 1 + echo See more info in the log in $tempf + echo + return 1 + else + rm $tempf + fi + else + echo Errors while checking URL "$1" + echo See the log in $tempf + return 1 + fi + url="$1" + shift + if wget "$@" "$url" ; then + return 0 + else + return 1 + fi +}
I'm still not sure if this is worth the complexity. Did you guys experience problems with 302 return codes?
Yes, that was the point of this change. For details see here: http://bugs.debian.org/515713 short: dns hijacks for web-based http logins for getting internet access seem to become more and more common these days. :-( -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4
participants (2)
-
Diego Biurrun -
Reinhard Tartler