| Message ID | 20130521153345.4880.68009.stgit@hds.com |
|---|---|
| State | New |
| Headers | show |
On 05/21/2013 09:33 AM, Tomoki Sekiyama wrote: > VSS SDK(*) setup.exe is only runnable on Windows. This adds a script > to extract VSS SDK headers on POSIX-systems using msitools. > > * http://www.microsoft.com/en-us/download/details.aspx?id=23490 > > From: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> > --- > scripts/extract-vsssdk-headers | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100755 scripts/extract-vsssdk-headers > > +#! /bin/bash Since you are using bash... > + > +# extract-vsssdk-headers > +# Author: Paolo Bonzini <pbonzini@redhat.com> > + > +set -e > +if test $# = 0 || ! test -f "$1"; then Why reject 0 arguments but not 2? Shouldn't the first check be test $# != 1? > + echo 'Usage: extract-vsssdk-headers /path/to/setup.exe' > + exit 1 > +fi > + > +# Extract .MSI file in the .exe, looking for the OLE compound > +# document signature. Extra data at the end does not matter. > +export LC_ALL=C > +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' > +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' ` ...I'd prefer $() over ``. > +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi > + > +# Now extract the files. > +tmpdir=tmp$$ > +mkdir $tmpdir ...also, this name is rather predictable; adding a $RANDOM might help improve its quality. > +msiextract -C $tmpdir vsssdk.msi > +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc > +rm -rf $tmpdir vsssdk.msi What, no trap, to guarantee clean up of the temp directory even if you Ctrl-C the script in the middle of a potentially long-running msiextract?
On 5/21/13 12:48 , "Eric Blake" <eblake@redhat.com> wrote: >On 05/21/2013 09:33 AM, Tomoki Sekiyama wrote: >> VSS SDK(*) setup.exe is only runnable on Windows. This adds a script >> to extract VSS SDK headers on POSIX-systems using msitools. >> >> * http://www.microsoft.com/en-us/download/details.aspx?id=23490 >> >> From: Paolo Bonzini <pbonzini@redhat.com> >> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> >> --- >> scripts/extract-vsssdk-headers | 25 +++++++++++++++++++++++++ >> 1 file changed, 25 insertions(+) >> create mode 100755 scripts/extract-vsssdk-headers >> > >> +#! /bin/bash > >Since you are using bash... > >> + >> +# extract-vsssdk-headers >> +# Author: Paolo Bonzini <pbonzini@redhat.com> >> + >> +set -e >> +if test $# = 0 || ! test -f "$1"; then > >Why reject 0 arguments but not 2? Shouldn't the first check be test $# >!= 1? I agree, will fix this. >> + echo 'Usage: extract-vsssdk-headers /path/to/setup.exe' >> + exit 1 >> +fi >> + >> +# Extract .MSI file in the .exe, looking for the OLE compound >> +# document signature. Extra data at the end does not matter. >> +export LC_ALL=C >> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' >> +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' ` > >...I'd prefer $() over ``. OK. >> +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi >> + >> +# Now extract the files. >> +tmpdir=tmp$$ >> +mkdir $tmpdir > >...also, this name is rather predictable; adding a $RANDOM might help >improve its quality. I will add $RANDOM here. >> +msiextract -C $tmpdir vsssdk.msi >> +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc >> +rm -rf $tmpdir vsssdk.msi > >What, no trap, to guarantee clean up of the temp directory even if you >Ctrl-C the script in the middle of a potentially long-running msiextract? OK, I will add some trap here. Maybe it also should have an additional check and a message to install Msitools for the case msiextract isn't exectable. Thanks,
On 05/21/13 23:02, Tomoki Sekiyama wrote: > Maybe it also should have an additional check and a message to install > Msitools for the case msiextract isn't exectable. Right. One POSIX-y way would be command -v msiextract >/dev/null and checking the exit status. http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html Alternatively, just go ahead calling it, and if the exit status is 126 or 127, assume it is not (correctly) installed and emit an extra hint afterwards: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01 Laszlo
On 05/21/13 17:33, Tomoki Sekiyama wrote: > VSS SDK(*) setup.exe is only runnable on Windows. This adds a script > to extract VSS SDK headers on POSIX-systems using msitools. > > * http://www.microsoft.com/en-us/download/details.aspx?id=23490 > > From: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com> > --- > scripts/extract-vsssdk-headers | 25 +++++++++++++++++++++++++ > 1 file changed, 25 insertions(+) > create mode 100755 scripts/extract-vsssdk-headers > > diff --git a/scripts/extract-vsssdk-headers b/scripts/extract-vsssdk-headers > new file mode 100755 > index 0000000..5877137 > --- /dev/null > +++ b/scripts/extract-vsssdk-headers > @@ -0,0 +1,25 @@ > +#! /bin/bash > + > +# extract-vsssdk-headers > +# Author: Paolo Bonzini <pbonzini@redhat.com> > + > +set -e > +if test $# = 0 || ! test -f "$1"; then > + echo 'Usage: extract-vsssdk-headers /path/to/setup.exe' > + exit 1 > +fi Error message to >&2 ? (Would not be worth a repost but you're already doing one...) > + > +# Extract .MSI file in the .exe, looking for the OLE compound > +# document signature. Extra data at the end does not matter. > +export LC_ALL=C > +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' Can't help mentioning the following portable (alas, octal) equivalent :) MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341') <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html#tag_20_94> > +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' ` Of these grep options, none are portable, hence I'm supposing dependency on GNU grep. In that case, see (*). > +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi In place of dd for seeking + cat, suggest tail -c +$((offset+1)) -- "$1" <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/tail.html> > + > +# Now extract the files. > +tmpdir=tmp$$ > +mkdir $tmpdir > +msiextract -C $tmpdir vsssdk.msi > +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc > +rm -rf $tmpdir vsssdk.msi > +exit 0 (*) Since we rely on GNU utilities anyway, I propose the mktemp utility ("Written by Jim Meyering and Eric Blake", heh) from GNU coreutils: tmpdir=$(mktemp -d) Feel free to ignore any of the above, of course :) Thanks, Laszlo
On 05/24/2013 07:38 AM, Laszlo Ersek wrote: >> +++ b/scripts/extract-vsssdk-headers >> @@ -0,0 +1,25 @@ >> +#! /bin/bash >> + >> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' > > Can't help mentioning the following portable (alas, octal) equivalent :) > > MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341') Yeah, but as long as the she-bang is (correctly) requiring bash, we might as well use the full power of bash. Also, Issue 8 of POSIX will be mandating $'' handling (but it could be several years before Issue 8 becomes a standard, compared to Issue 7 [aka POSIX 2008] just barely released its first technical corrigendum [aka POSIX 2013]).
On 05/24/13 17:59, Eric Blake wrote: > On 05/24/2013 07:38 AM, Laszlo Ersek wrote: > >>> +++ b/scripts/extract-vsssdk-headers >>> @@ -0,0 +1,25 @@ >>> +#! /bin/bash >>> + > >>> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' >> >> Can't help mentioning the following portable (alas, octal) equivalent :) >> >> MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341') > > Yeah, but as long as the she-bang is (correctly) requiring bash, Yes, that's why I edited my original "please consider using" to "can't help mentioning" :) I recalled that you had mentioned dash in one of your reviews (*), I checked your reply to this v3 03/11 and saw that it wasn't it -- here you'd written "Since you are using bash" so I edited the above paragraph. Clearly insufficiently :) (*) ... Apparently you mentioned dash in a MALLOC_PERTURB_ thread. L.
diff --git a/scripts/extract-vsssdk-headers b/scripts/extract-vsssdk-headers new file mode 100755 index 0000000..5877137 --- /dev/null +++ b/scripts/extract-vsssdk-headers @@ -0,0 +1,25 @@ +#! /bin/bash + +# extract-vsssdk-headers +# Author: Paolo Bonzini <pbonzini@redhat.com> + +set -e +if test $# = 0 || ! test -f "$1"; then + echo 'Usage: extract-vsssdk-headers /path/to/setup.exe' + exit 1 +fi + +# Extract .MSI file in the .exe, looking for the OLE compound +# document signature. Extra data at the end does not matter. +export LC_ALL=C +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1' +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' ` +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi + +# Now extract the files. +tmpdir=tmp$$ +mkdir $tmpdir +msiextract -C $tmpdir vsssdk.msi +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc +rm -rf $tmpdir vsssdk.msi +exit 0