bugGNU Octave - Bugs: bug #43979, test unpack fails in MXE-Octave...

 
 

bug #43979: test unpack fails in MXE-Octave ver 4.1.0+

Submitter:  Avinoam Kalma <avinoam>
Submitted:  Sun 11 Jan 2015 08:57:03 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Avinoam Open/Closed:  * Closed
Release:  * dev Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 18 Mar 2015 06:07:16 PM UTC, comment #10: 
Philip Nienhuis <philipnienhuis>
Group Member
Tue 17 Mar 2015 07:59:13 PM UTC, comment #9: 

See bugfix + patch in bug #44007, as these bugs are closely related.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 27 Jan 2015 09:01:46 PM UTC, comment #8: 

That's the same error I initially got in comment #1.

After the fix I couldn't reproduce it anymore, certainly not on Linux where it is in the "make check" test suite that I always run before "make all dist" & transplanting the dist to the mxe-octave build tree.

However, I can now confirm this for 4.1.0+ on Windows. I'll reopen the bug report, although this bug has a different cause.
Maybe next weekend I can have a closer look.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 19 Jan 2015 08:52:06 PM UTC, comment #7: 

test unpack failed for me (with the patch)


>> test unpack
***** test
 ## Create temporary directory and file for packing and unpacking
 dirname = tempname ();
 assert (mkdir (dirname));
 filename = tempname ();
 fid = fopen (filename, "wt");
 assert (fid >= 0);
 fprintf (fid, "Hello World\n");
 fprintf (fid, "123 456 789\n");
 fclose (fid);
 unwind_protect
   copyfile (filename, [filename ".orig"]);
   gzip (filename, dirname);
   [~, f] = fileparts (filename);
   filelist = unpack (fullfile (dirname, [f ".gz"]), P_tmpdir);
   assert (filelist{1}, filename);
   fid = fopen ([filename ".orig"], "rb");
   assert (fid >= 0);
   orig_data = fread (fid);
   fclose (fid);
   fid = fopen (filename, "rb");
   assert (fid >= 0);
   new_data = fread (fid);
   fclose (fid);
   if (orig_data != new_data)
     error ("unpack: Unpacked file does not equal original");
   endif
 unwind_protect_cleanup
   unlink (filename);
   unlink ([filename ".orig"]);
   rmdir (dirname);
 end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (filelist {1},filename)

  Location  |  Observed  |  Expected  |  Reason
     []      C:\Users\avinoam\AppData\Local\Temp\oct-PxjH0c\oct-HAjJsC
       C:\Users\avinoam\AppData\Local\Temp\oct-HAjJsC                    Strings don't match


Avinoam Kalma <avinoam>
Group Member
Wed 14 Jan 2015 08:51:28 PM UTC, comment #6: 
Philip Nienhuis <philipnienhuis>
Group Member
Wed 14 Jan 2015 05:37:22 AM UTC, comment #5: 

The change to make_absolute_filename should be okay.  I rewrote unpack recently and we used to rely on the fact that canonicalize_file_name returns "" if the file does not exist, but that is no longer the case.  I think you can commit the changeset.

Rik <rik5>
Group administrator
Mon 12 Jan 2015 11:09:42 PM UTC, comment #4: 

Just FTR, it is good to note here that unpack works fine as it is on Linux and Windows.

Depending on one's perception, the bug (a failing test) is:
- about one of the tests (i.e., merely cosmetic)
and/or
- about invoking the proper file separator on *nix and Windows systems.
Either perception is defendable I think, but the question is what the most practical solution would be.

make_absolute_filename() solves it, but perhaps the actual culprit is the test in question as it is naively expecting *nix style file separators even on Windows systems (due to canonicalize_file_name stubbornly returning forward slash fileseps).

Philip Nienhuis <philipnienhuis>
Group Member
Mon 12 Jan 2015 03:55:54 PM UTC, comment #3: 

here's the cs

(file #32811)

Philip Nienhuis <philipnienhuis>
Group Member
Mon 12 Jan 2015 03:54:41 PM UTC, comment #2: 

On linux, after changing canonicalize_file_name() by make_absolute_filename in L 147, all 9 tests still pass, so I think this fix is acceptable.

Nevertheless, I'd like the more experienced devs to have a look as I cannot oversee all consequences of this change.

Anyway, changeset attached.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 11 Jan 2015 09:42:16 PM UTC, comment #1: 

Thanks for the report.

Confirmed on 4.1.0+ & 3.9.0+

After stepping through the code I think the culprit is the call (at L.147) to canonicalize_file_name. That function converts native Windows backslash fileseps into forward slashes.

A similar alternative function (make_absolute_filename) leaves the file separators intact; but it has to be tested on Linux as well before that change can be made safely; as there seem to be some subtle differences between canonicalize_file_name and make_absolute_filename.  I had earlier discussion on this subject some years ago as regards drive letters on Windows (I see no unpack tests for that BTW), see here: https://savannah.gnu.org/bugs/?36677

Even then the test (first assert in unwind_protect block) fails for me:

>>   assert (filelist{1}, filename);

error: ASSERT errors for:  assert (filelist {1},filename)

  Location  |  Observed  |  Expected  |  Reason
     []      C:\Users\philip\AppData\Local\Temp\oct-31\oct-32
      C:\Users\philip\AppData\Local\Temp\oct-32                 Strings don't match


Philip Nienhuis <philipnienhuis>
Group Member
Sun 11 Jan 2015 08:57:03 PM UTC, original submission:  


Hi,
using GNU Octave Version: 4.1.0+ on Windows-7, (mxe-octave)
test unpack fails because file seperators difference:


>> test unpack
***** test
 ## Create temporary directory and file for packing and unpacking
 dirname = tempname ();
 assert (mkdir (dirname));
 filename = tempname ();
 fid = fopen (filename, "wt");
 assert (fid >= 0);
 fprintf (fid, "Hello World\n");
 fprintf (fid, "123 456 789\n");
 fclose (fid);
 unwind_protect
   copyfile (filename, [filename ".orig"]);
   gzip (filename, dirname);
   [~, f] = fileparts (filename);
   filelist = unpack (fullfile (dirname, [f ".gz"]), P_tmpdir);
   assert (filelist{1}, filename);
   fid = fopen ([filename ".orig"], "rb");
   assert (fid >= 0);
   orig_data = fread (fid);
   fclose (fid);
   fid = fopen (filename, "rb");
   assert (fid >= 0);
   new_data = fread (fid);
   fclose (fid);
   if (orig_data != new_data)
     error ("unpack: Unpacked file does not equal original");
   endif
 unwind_protect_cleanup
   unlink (filename);
   unlink ([filename ".orig"]);
   rmdir (dirname);
 end_unwind_protect
!!!!! test failed
ASSERT errors for:  assert (filelist {1},filename)

  Location  |  Observed  |  Expected  |  Reason
     []      C:/Users/avinoam/AppData/Local/Temp/oct-15                C:\Users\
avinoam\AppData\Local\Temp\oct-15                  Strings don't match


Thanks,

Avinoam

Avinoam Kalma <avinoam>
Group Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #32811:  unpack_bug43979.cs added by philipnienhuis (694B - text/x-csharp)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by avinoam (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 group members can vote.

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-03-18 philipnienhuis StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2015-03-17 philipnienhuis StatusConfirmed Patch Submitted
    2015-03-13 philipnienhuis Assigned toNone philipnienhuis
    2015-03-08 rik5 Dependencies- bugs #44445 is dependent
    2015-01-27 philipnienhuis StatusFixed Confirmed
        Open/ClosedClosed Open
    2015-01-14 philipnienhuis StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2015-01-12 philipnienhuis StatusPatch Submitted Ready For Test
    2015-01-12 philipnienhuis Attached File- Added unpack_bug43979.cs, #32811
    2015-01-12 philipnienhuis StatusConfirmed Patch Submitted
    2015-01-11 philipnienhuis StatusNone Confirmed
        Summarytest unapck fails in MXE-Octave ver 4.1.0+ test unpack fails in MXE-Octave ver 4.1.0+

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code