bugGNU Octave - Bugs: bug #45231, The run command does not return...

 
 

bug #45231: The run command does not return back to the originating directory

Submitter:  None
Submitted:  Mon 01 Jun 2015 10:17:02 PM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Incorrect Result
Status:  Fixed Assigned to:  None
Originator Name:  Tasos Papastylianou Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 4.0.0
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 10 Jun 2015 01:21:18 PM UTC, comment #12: 

Thanks for taking care of that Rik, lost track of this patch I had sitting around.

Mike Miller <mtmiller>
Group Member
Tue 09 Jun 2015 11:18:57 PM UTC, comment #11: 

I used the approach in comment #4 and checked in a change on the stable branch (http://hg.savannah.gnu.org/hgweb/octave/rev/c4f436483e49).  This will be a part of the 4.0.1 bug fix release.

Rik <rik5>
Group administrator
Fri 05 Jun 2015 03:32:23 PM UTC, comment #10: 

@Mike: I think it's okay to apply your suggested change from comment #4 to the stable branch.

Rik <rik5>
Group administrator
Wed 03 Jun 2015 01:12:30 AM UTC, comment #9: 

Thanks for jumping in Philip, I was looking at the changes you had made previously with make_absolute_filename, e.g. http://hg.savannah.gnu.org/hgweb/octave/rev/0cc52d752f99, and thought that would be the right fix here too, but I'm not familiar with the details of all of these various functions.

Mike Miller <mtmiller>
Group Member
Tue 02 Jun 2015 07:55:24 PM UTC, comment #8: 

@comment #5:
AFAICS make_absolute_filename() behaves better on Windows than canonicalize_file_name().

See (earlier posts in) bug #36677

Philip Nienhuis <philipnienhuis>
Group Member
Tue 02 Jun 2015 06:33:00 PM UTC, comment #7: 

@mtmiller: Confirming that

-      d = canonicalize_file_name (d);
+      d = make_absolute_filename (d);

resolves the issue.

@jwe Thanks; apologies, I didn't spot that option at the time!

Tasos Papastylianou <tpapastylianou>
Tue 02 Jun 2015 04:49:53 PM UTC, comment #6: 

Retagging release from 4.0.0-rc4 to 4.0.0.

John W. Eaton <jwe>
Group administrator
Tue 02 Jun 2015 03:41:13 PM UTC, comment #5: 

If the problem is with canonicalize_file_name() then we should report that to gnulib since we use their version in order to get cross-platform compatibility.  I suppose in the meantime it would be okay to switch over to Octave's own function of make_absolute_filename().

Rik <rik5>
Group administrator
Tue 02 Jun 2015 01:45:12 PM UTC, comment #4: 

Ok, that makes sense.

Can you see if the following change works for you instead:


diff --git a/scripts/miscellaneous/run.m b/scripts/miscellaneous/run.m
--- a/scripts/miscellaneous/run.m
+++ b/scripts/miscellaneous/run.m
@@ -61,7 +61,7 @@ function run (script)
   if (! isempty (d))
     if (exist (d, "dir"))
       startdir = pwd ();
-      d = canonicalize_file_name (d);
+      d = make_absolute_filename (d);
       unwind_protect
         cd (d);
         evalin ("caller", sprintf ("source ('%s%s');", f, ext),


Some work was done recently to make the make_absolute_filename better on Windows paths, and that should probably be used in this situation.

I can delete attachments, but we usually just ignore them.

Mike Miller <mtmiller>
Group Member
Tue 02 Jun 2015 09:04:04 AM UTC, comment #3: 

Hi Mike, thanks for the correction!

I was misled by the fact that strcmp(d,pwd) on line 70 of run.m was giving me 0, when it looked logical in the code that they should be the same, and they did look the same by eye at a quick glance! Sorry, I should have confirmed strcmp further!

The real culprit seems to be the canonicalize_file_name step on line 64, combined with my use of 'Unix-style' file-separators on a windows machine. This fails to correct the file separator just above the directory as you can see below


d   = C:\Users\tasos\Desktop/Child Directory
pwd = C:\Users\tasos\Desktop\Child Directory


therefore string comparison fails.

I fixed this on my system by adding a fullfile(d) instruction under line 64, but I suppose someone should really fix this directly in the built-in canonicalize_file_name function eventually!

Thanks for looking at this :)

PS. Is there a way to delete / strikethrough the previous patch?

(file #34148)

Tasos Papastylianou <tpapastylianou>
Tue 02 Jun 2015 02:53:41 AM UTC, comment #2: 

Thanks for the bug report and for coming up with a patch.

However, in the Octave language the strcmp function does return true when two strings are equal. Thanks for proposing a patch though! (mtmx on irc)

Do you have a strcmp function that returns 0 when the two arguments are equal? What do you get for the following?


>> strcmp (pwd, pwd)
ans =  1
>> which strcmp
'strcmp' is a built-in function from the file libinterp/corefcn/strfns.cc


Or if that's not the problem, do you get a different behavior when you use a directory without a space in it, just in case the space is causing problems?

Mike Miller <mtmiller>
Group Member
Mon 01 Jun 2015 11:26:26 PM UTC, comment #1: 

The bug seems to be on line 70 of the existing run.m command.
This uses strcmp within an if statement, in a way that seems to expect this to execute if the output of strcmp is true; however when two strings are equal, strcmp returns 0, therefore the if statement fails instead.

This is trivial to fix, but a patch is included below for consistency.

(file #34146)

Tasos Papastylianou <tpapastylianou>
Mon 01 Jun 2015 10:17:02 PM UTC, original submission:  

The 'run' command should enter a directory, run a script, then return to the calling directory. This is what is described in the documentation, and what would be compatible behaviour with the matlab equivalent.
Instead what happens is that it enters a directory, runs the script, and then stays there.

Trivial example:

pwd
mkdir 'Child Directory';
fh = fopen('./Child Directory/childscript.m','w');
fprintf(fh,'fprintf(''Hello from childscript\\n'');\n');
fclose(fh);
run './Child Directory/childscript';
pwd


Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #34148:  patch added by tpapastylianou (338B - application/octet-stream - Added fullfile(d) instruction under line 64 to correct wrong fileseparator resulting from use of canonicalize_file_name on windows)
file #34146:  patch added by tpapastylianou (395B - application/octet-stream - Patch: Fixes line 70 of existing run.m to correct strcmp behaviour)

 

Depends on the following items: None found

Digest:
   bug dependencies.

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by tpapastylianou (Updated the item)
  • -email is unavailable- added by None (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-02 rik5 Dependencies- bugs #48351 is dependent
    2015-06-09 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2015-06-02 jwe Release4.0.0-rc4 4.0.0
    2015-06-02 mtmiller StatusWorks For Me Confirmed
    2015-06-02 tpapastylianou Attached File- Added patch, #34148
    2015-06-02 mtmiller StatusNone Works For Me
    2015-06-01 tpapastylianou Attached File- Added patch, #34146

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code