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 FunctionSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Incorrect Result
Status: FixedAssigned to: Philip Nienhuis <philipnienhuis>
Originator Name: AvinoamOpen/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)

Wed 18 Mar 2015 06:07:16 PM UTC, comment #10:

Fixed in this cs:
http://hg.savannah.gnu.org/hgweb/octave/rev/8c20fb6caa16

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)

Avinoam Kalma <avinoam>
Project Member
Wed 14 Jan 2015 08:51:28 PM UTC, comment #6:

pushed
http://hg.savannah.gnu.org/hgweb/octave/rev/0cc52d752f99

Closing report

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:

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:

Thanks,

Avinoam

Avinoam Kalma <avinoam>
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 #32811:  unpack_bug43979.cs added by philipnienhuis (694B - text/x-csharp)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by avinoam (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 14 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 18 Mar 2015 06:07:16 PM UTCphilipnienhuisStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Tue 17 Mar 2015 07:59:13 PM UTCphilipnienhuisStatusConfirmed=>Patch Submitted
    Fri 13 Mar 2015 05:40:59 PM UTCphilipnienhuisAssigned toNone=>philipnienhuis
    Sun 08 Mar 2015 01:38:10 AM UTCrik5Dependencies-=>bugs #44445 is dependent
    Tue 27 Jan 2015 09:01:46 PM UTCphilipnienhuisStatusFixed=>Confirmed
      Open/ClosedClosed=>Open
    Wed 14 Jan 2015 08:51:28 PM UTCphilipnienhuisStatusReady For Test=>Fixed
      Open/ClosedOpen=>Closed
    Mon 12 Jan 2015 03:56:20 PM UTCphilipnienhuisStatusPatch Submitted=>Ready For Test
    Mon 12 Jan 2015 03:55:54 PM UTCphilipnienhuisAttached File-=>Added unpack_bug43979.cs, #32811
    Mon 12 Jan 2015 03:54:41 PM UTCphilipnienhuisStatusConfirmed=>Patch Submitted
    Sun 11 Jan 2015 09:42:16 PM UTCphilipnienhuisStatusNone=>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.1-cleanup1