bugGNU Octave - Bugs: bug #61166, mkdir(parent, dirname) fails if...

 
 

bug #61166: mkdir(parent, dirname) fails if parent does not exist

Submitter:  Sébastien Villemot <svillemot>
Submitted:  Wed 15 Sep 2021 04:17:54 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Open/Closed:  * Closed
Release:  * 6.3.0 Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 19 Apr 2022 10:32:26 PM UTC, comment #11: 

No responses.  Assuming patches work as expected.  Marking as Fixed and closing report.

Rik <rik5>
Group administrator
Mon 27 Sep 2021 09:54:46 PM UTC, comment #10: 

Oops, I thought I ran the tests...

Anyway, I think the test in mkdir that was previously expected to fail should actually pass now.  I removed the test from mkdir.m and added some more tests to system.tst that use relative file names.

John W. Eaton <jwe>
Group administrator
Mon 27 Sep 2021 09:28:20 PM UTC, comment #9: 

processing /home/buildbotu/fc25-x86_64/gcc-fedora/src/scripts/miscellaneous/mkdir.m
*** test <*55540>
 fail ('mkdir ("__%hello%__", "world")', "invalid PARENT");
!!!!! regression: https://octave.org/testfailure/?55540
expected error <invalid PARENT> but got none

Dmitri A. Sergatskov <dasergatskov>
Mon 27 Sep 2021 07:16:22 PM UTC, comment #8: 

I added some tests to my proposed patch and pushed the following changeset:

http://hg.savannah.gnu.org/hgweb/octave/rev/54520422f056

Marking report as ready for test.

John W. Eaton <jwe>
Group administrator
Tue 21 Sep 2021 03:30:05 PM UTC, comment #7: 

I didn't look at their code. But I guess gnulib is using the Windows API that MS calls "ANSI".
That takes `char *` for strings. These are interpreted in the system's (8 bit) locale encoding. Not all characters can be represented in all locales.
There is another variant of the Windows API that MS calls "Unicode".
That takes `wchar_t *` for strings. Those are 16 bit wide on Windows. They are required to be encoded in UTF-16 (not locale dependent).

Octave stores strings internally in UTF-8 wherever possible. We are converting those to UTF-16 and use the Unicode Windows API in our code.
With that approach, we are able to access files or folders that contain characters that cannot be represented in the system's locale (we had bug reports about that in the past that imprinted in memory).
If we don't want to lose that, we'd need some interface changes to the gnulib functions.

One possible way could be that gnulib required UTF-8 encoded strings on Windows (always). But that would break existing code if it relies on `char *` strings being encoded in the system's locale encoding.
Another possibility would be that gnulib duplicated their functions à la MS and added functions that accepted `wchar_t *`.
Yet another possibility might be that they kept the functions with the current API (i.e., locale encoded `char ` strings) and added new modules that implement the same functions but using UTF-8 encoded `char ` strings (and the Unicode API on Windows). We could select to use these UTF-8 modules.

None of those alternatives seem likely to happen.


Your changeset looks good to me and seems to work on Windows 10. It fixes the example in comment #0.

Markus Mützel <mmuetzel>
Group administrator
Tue 21 Sep 2021 01:41:40 PM UTC, comment #6: 

Is there anything we could do to fix the gnulib functions to do the right thing for us?

Is the change I attached in comment #2 acceptable?

John W. Eaton <jwe>
Group administrator
Thu 16 Sep 2021 08:13:51 AM UTC, comment #5: 

OT: If we want to support non-ASCII characters in the file system on Windows, unfortunately we can't use gnulib functions for any file system related operations (on that platform). The reason is that gnulib has no wide character API (and no API that enforces UTF-8) and Windows doesn't support a Unicode narrow character locale (or rather its implementation breaks many existing programs).

Markus Mützel <mmuetzel>
Group administrator
Wed 15 Sep 2021 06:13:27 PM UTC, comment #4: 

"why? who knows"

For sure, I wasn't expecting an actual answer.  :-)

John W. Eaton <jwe>
Group administrator
Wed 15 Sep 2021 05:53:17 PM UTC, comment #3: 

why? who knows. assuming it's that way for matlab compatibilty - Matlab errors out with more than two folders given:

+matlab+

>> mkdir a b c

Error using mkdir
Too many input arguments.
-matlab-

no reason I can think of that we couldn't accept more than 2 folders if we really wanted that syntax for some reason.

Nicholas Jankowski <nrjank>
Group Member
Wed 15 Sep 2021 05:35:46 PM UTC, comment #2: 

Changing the item group to incorrect result.  Octave's own documentation says


Create a directory named DIRNAME in the directory PARENT, creating any intermediate directories if necessary.


so I guess that could be interpreted as saying that any  missing parts of "fullfile (parent, dirname)" should be created.

It seems a bit weird to accept a single parent directory name.  Why not just require


mkdir foo/bar/...


Or, why just one parent?  Why not


mkdir foo bar baz ...


Anyway, what about the attached patch?  It could also probably use a few tests for missing directories, parent names with "~", etc., especially since this function may eventually be replaced with code from gnulib?  So maybe those tests should go in the test directory instead of in mkdir.m so they aren't accidentally lost in the future if mkdir.m is removed.

(file #51923)

John W. Eaton <jwe>
Group administrator
Wed 15 Sep 2021 05:02:14 PM UTC, comment #1: 

confirming with Matlab 2021a

if neither are present

mkdir foo bar

results in ./foo/bar

Octave 6.3.0 produces:


>> mkdir foo bar
error: mkdir: invalid PARENT
error: called from
    mkdir>mkdir_recur at line 90 column 5
    mkdir>mkdir_recur at line 95 column 26
    mkdir at line 72 column 21


Nicholas Jankowski <nrjank>
Group Member
Wed 15 Sep 2021 04:17:54 PM UTC, original submission:  

The following invocation of the 2-argument syntax for mkdir:

mkdir foo bar

fails if “foo” does not exist.

It works on MATLAB.

Sébastien Villemot <svillemot>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51923:  mkdir-diffs.txt added by jwe (828B - text/plain)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by dasergatskov (Posted a comment)
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by nrjank (Posted a comment)
  • -email is unavailable- added by svillemot (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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-04-19 rik5 StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2021-09-27 jwe StatusPatch Submitted Ready For Test
    2021-09-21 jwe StatusConfirmed Patch Submitted
    2021-09-15 jwe Attached File- Added mkdir-diffs.txt, #51923
        Item GroupMatlab Compatibility Incorrect Result
    2021-09-15 nrjank StatusNone Confirmed

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code