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

 
 

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

Submitted by:  Avinoam Kalma <avinoam>
Submitted on:  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:  Philip Nienhuis <philipnienhuis>
Originator Name:  Avinoam Open/Closed:  Closed
Release:  dev Operating 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)

Wed 18 Mar 2015 06:07:16 PM UTC, comment #10: 
Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project Member
Wed 14 Jan 2015 08:51:28 PM UTC, comment #6: 
Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
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>
Project 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>
Project MemberIn charge of this item.
Mon 12 Jan 2015 03:55:54 PM UTC, comment #3: 

here's the cs

(file #32811)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project MemberIn charge of this item.
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>
Project 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)
  •  

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

    Only project 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.5