Incorrect handling of filenames with spaces

Bug #180223 reported by Davide Capodaglio on 2008-01-03
8
Affects Status Importance Assigned to Milestone
autopano-sift (Ubuntu)
Low
Unassigned

Bug Description

Binary package hint: autopano-sift

autopano-complete.sh script does not correctly handles filenames or directories with spaces, and so it is not possible to automatically run the autopano-sift script from hugin.
Since it is very common to rename photo album folder with names, dates, ecc, this is very annoying.

Davide Capodaglio (davidecapod) wrote :

I attach a possible patch to handle both directories with spaces and filenames with spaces.
It uses explicitly bash since it requires arrays.

miji2 (miji22) wrote :

Definitely affected by the same bug. Was about to file this bug as well. My fix was very similar to Davide.

miji2

Daniel Holbach (dholbach) wrote :

Is there no way to make it not bash-centric?

Francesco Potortì (pot) wrote :

I patched autopano, autopanog, autopano-complete, autopano-complete.old for using spaces in file and directory names.
I only used Bourne shell constructs, so it should work with any shell.
By the way, both autopano-complete and autopano-complete.old contained bashisms, that is, even if they have a #! /bin/sh in the first line, they used the $() construct, which is not legal in Bourne shell; I corrected this.
As a general rule, only write bash scripts using file names if you know well what you are doing: correct treatment of generic file names in Bourne shell or Bash is tricky. Please let me know if you encounter problems with this patch.

In the attachment, together with a diff and the original files, you can find the patched files, which you can copy to your /usr/bin directory. Specifically, copy autopano/patched/autopano-complete to /usr/bin/autopano-complete in order to automatically generate points with Hugin.

Francesco Potortì (pot) wrote :

I can confirm the bug in autopano-sift (2.4-0ubuntu6) gutsy. A patch is available.

Changed in autopano-sift:
status: New → Confirmed
Davide Capodaglio (davidecapod) wrote :

The proposed patch does not work if both the directory AND the file name contain spaces; it only works if the directory contains them.

Francesco Potortì (pot) wrote :

Grumpf. Sorry, I thought I had checked even spaces in file names, but I did not...
Here is a corrected version. Only autopano-complete was affected by the bug, and now it works for me. As before, please let me know if you encounter problems with this patch.

Davide Capodaglio (davidecapod) wrote :

Francesco, do you know if someone notified this also upstream?
When I opened this bug I also sent an email to <email address hidden>, but got no answer...

Francesco Potortì (pot) wrote :

>Francesco, do you know if someone notified this also upstream?

Io no.

>When I opened this bug I also sent an email to <email address hidden>, but got no answer...

Magari riprovaci. Gli antispam possono fare scherzi...

Francesco Potortì (pot) wrote :

Ops, pardon...

>Francesco, do you know if someone notified this also upstream?

Not me.

>When I opened this bug I also sent an email to <email address hidden>, but
got no answer...

Maybe try again? Sometimes antispam plays tricks...

Sebastian Nowozin answered me:

The fix looks fine, and I will apply it to my source tree. However, there will probably be no further release of autopano-sift. The good news is that the hugin people are working on an integrated feature extraction program, so autopano-sift can slowly retire.

So to see your patch in Hardy it must be approved with Ubuntu patch procedure...

I am taking care of creating a debdiff from this patch.

Changed in autopano-sift:
assignee: nobody → christophe.sauthier
xtknight (xt-knight) wrote :

Here is a debdiff which might fix the problem. I used Francesco Potortì's patch (thanks). I had to make some adaptations to make it work with Ubuntu.

There's two autopano-complete.old.sh files in Ubuntu, and three autopano-complete.sh files.

src/util/autopano-complete.sh was an oddball. It didn't exactly match what his patch specified so I took a guess and made changes that seemed pertinent and consistent with the rest of the files.

It referred to MONO=./monoopt.sh

So I looked in ./monoopt.sh and also made the following change:

mono --optimize=peephole,branch,inline,cfold,consprop,copyprop,deadce,linears,cmov,intrins,tailc,loop,fcmov,leaf \
- $*
+ "$@"

Not sure if that's needed but doesn't look harmful anyway.

Please test the patch. I have the attached debdiff on my PPA as autopano-sift 2.4-0ubuntu7~ppa1 right now.
https://launchpad.net/~xt-knight/+archive

Changed in autopano-sift:
assignee: christophe.sauthier → nobody
Francesco Potortì (pot) wrote :

>Please test the patch

I tried to install the .deb, but it is made for Hardy, and I have
Gutsy. So either I upgrade to Hardy (is that stable enough?) or you
suggest me a way to apply your patches to try out.

Maybe simplest would be for you to send me a tar containing all the
files you patched, so that I can overwrite the shell scripts affected.

xtknight (xt-knight) wrote :

Francesco: well it's really preferred that we test the Hardy one. We will have to sometime. We can't get it into Gutsy if it is not tested in Hardy.

You don't have to test it, anyone is free to. But if you want to accelerate the process feel free. :) You can install Hardy inside a virtual machine so it can not harm your data.

Francesco Potortì (pot) wrote :

>Francesco: well it's really preferred that we test the Hardy one. We
>will have to sometime. We can't get it into Gutsy if it is not tested
>in Hardy.

The patches I wrote are against shell scripts. Unless you modified
other things, such patches are not dependent on system version (and are
independent of architecture, too). So testing them on any system should
be allright.

Anyway, if anyone gets to test them, they should be tested against both
file names and directory names containing spaces.

xtknight (xt-knight) wrote :

Francesco:

Here's the Gutsy debdiff but I doubt it's worth filing an SRU for it.

You're right though. If this Gutsy one works for you, the Hardy one will work. The packages were nearly identical. I didn't even make any changes to the actual patch at all.

Gutsy test packages should be available at my PPA soon.

https://launchpad.net/~xt-knight/+archive

You can click on the autopano-sift that says Gutsy at the right.

autopano-sift (2.4-0ubuntu8) intrepid; urgency=low

  * debian/patches/03_autopano_filenames_with_spaces.diff :
    Allow autopano to support filenames with spaces. (LP: #180223)
    Thanks to Andy Matteson and Francesco Potortì.

 -- Christophe Sauthier <email address hidden> Tue, 20 May 2008 17:57:41 +0200

Emilio Pozuelo Monfort (pochu) wrote :

Uploaded, thanks for your work!

Changed in autopano-sift:
importance: Undecided → Low
status: Confirmed → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package autopano-sift - 2.4-0ubuntu8

---------------
autopano-sift (2.4-0ubuntu8) intrepid; urgency=low

  * debian/patches/03_autopano_filenames_with_spaces.diff :
    Allow autopano to support filenames with spaces. (LP: #180223)
    Thanks to Andy Matteson and Francesco Potortì.

 -- Christophe Sauthier <email address hidden> Tue, 20 May 2008 17:57:41 +0200

Changed in autopano-sift:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers