bugQuilt - Bugs: bug #25579, quilt setup does not work when...

 
 

bug #25579: quilt setup does not work when tarball contains directory with space

Submitter:  Pavol Rusnak <stick>
Submitted:  Fri 13 Feb 2009 02:34:22 PM UTC
   
 
Category:  None Severity:  2 - Minor
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  khali
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 17 Feb 2012 08:25:33 PM UTC, comment #7: 

Fix committed:

http://git.savannah.gnu.org/cgit/quilt.git/commit/?id=07a1ee5d0bcd6ac3f860b3cf6c56515a7aa81d63

together with an improvement to the test suite to test for this case.

Jean Delvare <khali>
Group administrator
Thu 02 Feb 2012 07:44:56 PM UTC, comment #6: 

I'm all for fixing the problem completely, just I'm not sure what problem you're talking about. "read" doesn't treat every non-alphanumeric character specially by default, only space, tab and newline. So only space, tab and newline need escaping as far as I can see. My proposal only handles space simply because I just don't expect people to use tabs or newlines in their directory or archive names. It would be easy enough to set IFS to only space if that really bothers you.

Oh, and backslash would need escaping too, granted, but then again I don't really expect people to use that in file or directory names.

As far as a test case is concerned, I have that already, and I'll commit it together with the fix. It tests for spaces in archive and directory names as well as square brackets, and my second proposal passes that. I can add more characters in the test if you have specific concerns.

Jean Delvare <khali>
Group administrator
Thu 02 Feb 2012 06:58:53 PM UTC, comment #5: 


> Escaping spaces seems enough


Well, why shouldn't we fix this kind of problem completely if we can?  I've even written a slightly optimized quoing function already that should just work.

Other than that, the patch looks simple enough, but a test case would still be great.

Andreas Gruenbacher <agruen>
Group administrator
Thu 02 Feb 2012 06:48:41 PM UTC, comment #4: 

Oh, I had not considered this approach, thanks for the pointer. Then I don't quite get why we would have to escape all "bad" characters... Escaping spaces seems enough to fix this bug and it is straightforward. See new attachment.


(file #24959)

Jean Delvare <khali>
Group administrator
Thu 02 Feb 2012 01:20:43 PM UTC, comment #3: 

The output of "inspect" is parsed by bash "read" which treats backslash as an escape character, so we only need to backslash escape bad characters in the output of "inspect".  Something like this might work:

backslash_escape() {
        good='-._+/A-Za-z0-9'

        if [ -z "${1//[$good]/}" ]
        then
                echo "$1"
        else
                echo "$1" | sed -e "s/\([^$good]\)/\\\\\\1/g"
        fi
}

Andreas Gruenbacher <agruen>
Group administrator
Thu 02 Feb 2012 09:09:05 AM UTC, comment #2: 

I have a fix for this, see attachment. It adds some complexity to the code and is not perfect. Now we support space in directory names, but we no longer support "|" as I used this character as a separator. Not that I really expect this character to be found in directory names, but I did not really expect spaces to be found either. Also, we still do not support space in patch file names (and probably never will.)

Opinions whether this patch should be applied or not are welcome.


(file #24956)

Jean Delvare <khali>
Group administrator
Mon 16 Feb 2009 11:52:08 AM UTC, comment #1: 

Quilt uses /usr/share/quilt/scripts/inspect for analyzing spec files, which reports what's in the spec file in a line based format like:

    tar . package-1.2.3.tar.gz
    patch package-1.2.3 patch.diff

this format would need to be changed.

Quilt can't deal with spaces in patch names either, and quite a bit of cleaning up would be required to fix that.

Andreas Gruenbacher <agruen>
Group administrator
Fri 13 Feb 2009 02:34:22 PM UTC, original submission:  

When my package contains tarball with directory containing space (eg. "Frets on Fire") - quilt setup *spec fails. It tries to create directory patches in Frets/patches not "Frets on Fire/patches"

Pavol Rusnak <stick>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #24959:  quilt-setup-support-spaces-2.patch added by khali (1KiB - text/x-patch - Alternative fix)
file #24956:  quilt-setup-support-spaces.patch added by khali (2KiB - text/x-patch - Candidate fix)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by khali (Updated the item)
  • -email is unavailable- added by agruen (Posted a comment)
  • -email is unavailable- added by stick (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2012-02-17 khali StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2012-02-02 khali Attached File- Added quilt-setup-support-spaces-2.patch, #24959
    2012-02-02 khali Attached File- Added quilt-setup-support-spaces.patch, #24956
        Assigned toNone khali
    2009-02-16 agruen Severity3 - Normal 2 - Minor
        StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-4b48.
    Corresponding source code