bugGNU Octave - Bugs: bug #47609, zip.m: errrors when zipping to a...

 
 

bug #47609: zip.m: errrors when zipping to a directory with spaces in name on Windows

Submitted by:  Philip Nienhuis <philipnienhuis>
Submitted on:  Sat 02 Apr 2016 10:40:23 PM UTC  
 
Category: Octave FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: None
Originator Name: Philip NienhuisOpen/Closed: Closed
Release: devOperating System: Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Mon 04 Apr 2016 05:34:32 PM UTC, comment #12:

I agree, this will probably confuse some Windows users and should probably be mentioned someplace, I don't know what the right place is :)

Closing this one as fixed.

Mike Miller <mtmiller>
Project Administrator
Mon 04 Apr 2016 05:21:07 PM UTC, comment #11:

@comment #10:
Sure, but where?
In the various texinfo headers?
In the manual?
On the wiki? (FAQ?)

One might argue that as this wildcard behavior is a MingW/MSYS issue it had better be outlined in a general place.

My point is merely that most Windows users probably do not expect *nix behavior on a Windows box. They're probably not even aware that wildcard treatment is different from other, "normal" Windows programs.
Too bad I haven't got the experimental MSVC binaries lying around that Michael Goffioul once made, to check how this worked with an Octave built with native Windows tools.
It's worthwile to check with Matlab as well; must happen at work as the prerelease timed out.

Philip Nienhuis <philipnienhuis>
Project Member
Mon 04 Apr 2016 04:29:48 PM UTC, comment #10:

Just pushed:

http://hg.savannah.gnu.org/hgweb/octave/rev/3b9524234d89

Want to tackle the documentation issues of how to teach Windows users how wildcards work in a separate bug report?

Mike Miller <mtmiller>
Project Administrator
Mon 04 Apr 2016 04:26:36 PM UTC, comment #9:

@Philip: Yes, cygwin is very annoying in that it returns true for both ispc and isunix. A true Windows machine is

Rik <rik5>
Project Administrator
Mon 04 Apr 2016 04:00:23 PM UTC, comment #8:

Is the "! isunix ()" required for avoiding cygwin? (i.e., do ispc() and isunix() both return true on cygwin?)

Anyway the patch looks fine to me. Will you push or shall I?

Philip Nienhuis <philipnienhuis>
Project Member
Sun 03 Apr 2016 08:41:46 PM UTC, comment #7:

Right, setting the documentation of wildcards aside for now, how about this revision of your patch?

(file #36829)

Mike Miller <mtmiller>
Project Administrator
Sun 03 Apr 2016 07:06:57 PM UTC, comment #6:

As far as my experience with MSYS/mingw goes, yes this wildcard "issue" (idiosyncrasy) is valid for all utilities. So probably unpack / gzip / glob / dir could also use relevant information in the texinfo header. See below for dir and glob:

ls seems not affected:

But then one gets the question whether this should be mentioned in those texinfo headers, or in a more general place in the manual (with more chance of being overlooked by users), and/or in the FAQ ?

Philip Nienhuis <philipnienhuis>
Project Member
Sun 03 Apr 2016 05:26:28 PM UTC, comment #5:

I agree that it would be better to not "fix" the wildcard automatically but document it. My main question is, doesn't this affect every command that accepts wildcards? Such as dir, ls, glob, gzip, zip, unpack?

My point in comment #2 was because I got confused with all the permutations. Indicating the path separately works because Octave knows how to chdir to that path and the zip command with argument "" then succeeds. I thought I had seen you post an example that worked with a "backslashed\path\" argument, but I was wrong. So yes, I agree this seems like a bug with the zip executable that we should work around as you indicated.

The condition should be

as used elsewhere in scripts.

Mike Miller <mtmiller>
Project Administrator
Sun 03 Apr 2016 04:55:48 PM UTC, comment #4:

@comment #3:
*.* on Windows is usually meant to catch all files and subdirs, with or without extension (on Windows files and directories can have extensions; extensions of directories are not special, just part of a name).
That works differently on nix - the period between file/directory name and extension isn't special on nix (and msys/mingw and cygwin), while it is on Windows (that is, Windows utilities and -programs find it special => Windows users perceive it as special too).
I found that *.* even works adversely on mingw, see the first two verbatim blocks in comment #1, hence my advice in the texinfo header in the changeset.

Of course zip.m can have stanza along the lines of:

but I think it is better to have users know what they're doing than have programs guessing user's intentions. After all, it may be possible that a literal *.* is required and/or intended.

@comment #2:
The cmd lines obtained in zip.m that are conveyed to zip.exe are in the 5th verbatim block of comment #1.

The purpose of my cset is simply to ensure properly zipping to directories with spaces in the name using zip.m on Windows systems.
On Linux, zip.m and friends work as advertised, probably due to Rik's recent work.

Backslashes give problems when fed to zip(.exe), see the 5th verbatim block in comment #1 (debugging zip.m on Windows).
Several mingw utilities behave inconsistently as regards file separator styles. zip.exe is a fine example: it does not error on backslashes in its first argument (the zip file) but does in the second (the files to be zipped). I did not include that in the verbatim block, but that's what I got while experimenting.

One can argue that backslash behavior had rather be fixed upstream, but I think for now it is vastly better to simply avoid backslash fileseps at all when dealing with mingw utilities. All of those those work fine with forward slash fileseps.

Philip Nienhuis <philipnienhuis>
Project Member
Sun 03 Apr 2016 02:57:57 PM UTC, comment #3:

Is the

wildcard typically expected to match directory names with no extension on Windows? What about files with no extension?

Mike Miller <mtmiller>
Project Administrator
Sun 03 Apr 2016 02:56:52 PM UTC, comment #2:

Is the . wildcard expected to match directory names with no extension in Windows typically? That seems like a more generic problem/misunderstanding not specific to zip.

I got some errors due to paths with spaces in Debian with 4.0.1, but not with the default branch:

So no errors due to spaces here, and no errors due to spaces in Philip's examples either, just errors due to backslash file separator.

That's suspicious that the same path results in no error in the previous example. How is that working? What is cmd in the case where it does work and why does that work and not the other?

Mike Miller <mtmiller>
Project Administrator
Sat 02 Apr 2016 10:53:37 PM UTC, comment #1:

It seems the bugreport didn't get uploaded in the form I hoped it would be :-(

Another try:

Files to zip are in:
C:\Users\philip\AppData\Local\Temp\oct-dIRfU3

File to zip to:
c:\tmp\spaces in name\test.zip

Works partly. dir apparently does not understand the traditional Windows wildcard ..
It only copied the one file in the root of ....\oct-dIRfU3 but not the subdirs

This ^^ works fine.

Does not work at all.

Same, don't work.

Debugging zip.m:

Conclusions:
- In zip.m, Windows users should be informed that "*.*" wildcard doesn't work as expected on Windows, they should use "*" instead;
- Ditto, the FILES argument should have forward slash fileseps

Changeset for these two attached.

(file #36825)

Philip Nienhuis <philipnienhuis>
Project Member
Sat 02 Apr 2016 10:40:23 PM UTC, original submission:

Some investigation after bug #47578 shows the following:

Files to zip are in:
C:\Users\philip\AppData\Local\Temp\oct-dIRfU3

File to zip to:
c:\tmp\spaces in name\test.zip

Philip Nienhuis <philipnienhuis>
Project Member

 

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

Attach File(s):
   
   
Comment:
   

Attached Files
file #36829:  bug-47609-2.diff added by mtmiller (947B - text/x-diff)
file #36825:  bug_47609.cset added by philipnienhuis (1KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by philipnienhuis (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 5 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 04 Apr 2016 05:34:32 PM UTCmtmillerStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Sun 03 Apr 2016 08:41:46 PM UTCmtmillerAttached File-=>Added bug-47609-2.diff, #36829
      StatusNone=>Patch Submitted
    Sat 02 Apr 2016 10:53:37 PM UTCphilipnienhuisAttached File-=>Added bug_47609.cset, #36825

    Back to the top


    Powered by Savane 3.1-cleanup1