Hi KiBi,
On Mon, Jul 07, 2014 at 03:56:21AM +0200, Cyril Brulebois wrote:
> Modestas Vainius <modax@debian.org> (2013-12-29):
>
> thanks for the patch but I'm not convinced, see below:
>
> > --- a/debian/iso-scan.postinst
> > +++ b/debian/iso-scan.postinst
> > @@ -162,7 +162,7 @@ scan_device_for_isos() {
> > elif [ "$look_subdirs" = 1 ]; then
> > opt="-type f"
> > fi
> > - isolist=$(find $dir $opt -name "*.iso" -o -name "*.ISO" 2>/dev/null)
> > + isolist=$(find "$dir" $opt -name "*.iso" -o -name "*.ISO" 2>/dev/null)
>
> This part is certainly OK; at least I can't think of a reason why that
> wouldn't be a good thing.
>
> > TOPLEVEL_DIRS_COUNT=$(($TOPLEVEL_DIRS_COUNT + 1))
> >
> > for iso in $isolist; do
>
> but then that means we're possibly going to fail here. Example:
[snip]
> I guess it would make sense to fix this for real instead of hiding it a
> bit further. Unfortunately 4am isn't a great time to set up a reproducer
> and to keep on hacking. :/
There are in effect two bugs here. The first, which the patch fixes,
is that any folder with spaces in its name will cause iso-scan's
postinst to fail, preventing the installation. The second, which the
patch doesn't fix, is that any ISO found in a folder with spaces in
its name won't be handled correctly.
The first bug is extremely confusing, since an otherwise OK USB key
with all the appropriate files in the right place will fail to
install, with no explicit error message, and worse than that with a
message on the fourth terminal indicating that the ISO was found and
is usable...
The second bug, which is mitigated in part by the existence test (line
170), will only prevent certain ISOs from being used, and won't abort
the installation.
Wouldn't it be acceptable to apply the patch, and add an erratum
indicating that ISOs shouldn't be placed in folders containing spaces
in their names?
Regards,
Stephen
Attachment:
signature.asc
Description: Digital signature