bugGNU Octave - Bugs: bug #65559, savepath() regression no longer...

 
 

bug #65559: savepath() regression no longer saves paths

Submitter:  Dimitry <dchuprakov>
Submitted:  Fri 05 Apr 2024 11:01:49 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  Dimitry Chuprakov Open/Closed:  * Closed
Release:  * 9.1.0 Operating System:  * Any
Fixed Release:  9.2.0 Planned Release:  9.2.0
* Mandatory Fields

Post a Comment

Add a New Comment Rich Markup
   

Discussion

Jump to the original submission

Fri 12 Apr 2024 04:00:12 PM UTC, comment #24: 

While I don't think it's great to throw a "warning" message, I also think there are many other bugs which are a higher priority to resolve.  Since regression has been fixed I'm going to close this report.

Rik <rik5>
Group administrator
Tue 09 Apr 2024 06:08:56 PM UTC, comment #23: 

Afaict, warnings can be silenced with `warning ("off", WARN_ID)`. I guess that's the reason why jwe used a warning here in the first place.

`savepath` is an old function that also exists in Matlab:
https://www.mathw ... ref/savepath.html

Extending syntax bit us a couple of times in the past. But that specific function is very old and Matlab didn't change it yet. (Not sure if that says anything for the future though.)

If you really object to the warning, I guess we could extend the syntax...

Markus Mützel <mmuetzel>
Group administrator
Tue 09 Apr 2024 05:52:55 PM UTC, comment #22: 

The message is displayed regardless (no way to silence) which is probably not a good idea nor a good architecture.

Instead of always printing the message, I think it would be a better idea to change the function signature so that there is a second return argument.  For example, the function signature for fcntl() is


[STATUS, MSG] = fcntl (FID, REQUEST, ARG)


Lots of the syscall functions work this way where the first return argument is status and an additional argument is extra information.

Rik <rik5>
Group administrator
Tue 09 Apr 2024 05:32:38 PM UTC, comment #21: 

That looks good to me. 👍

Is there a way to silence the output in the test with that idiom though?

Markus Mützel <mmuetzel>
Group administrator
Tue 09 Apr 2024 05:00:57 PM UTC, comment #20: 

My objection is to the "warning:" prefix and the implication that something has gone wrong.  If this is a useful informational warning to keep then code like this might be more appropriate.


fprintf ("savepath: current path saved to %s", file);
fflush (stdout);


Rik <rik5>
Group administrator
Tue 09 Apr 2024 03:49:48 PM UTC, comment #19: 

My remark was rather for the prompt which disappears after clicking on Save in the Set Path dialog.

  Edit menu -> Set Path -> Save (-> Close)

But this is a detail.

Charles Praplan <charprap>
Tue 09 Apr 2024 03:14:23 PM UTC, comment #18: 

@jwe: See comment #10, comment #15, comment #16.

Octave emits a warning for something which is purely informational, and the function is behaving as documented.  I would propose that this be removed, but I may not understand why this was added.  Using "hg blame" the code is


 7392:   unwind_protect_cleanup
30875:     sts = fclose (fid);
30875:     if (sts < 0)
11469:       error ("savepath: could not close savefile after writing, %s", file);
 7392:     elseif (nargin == 0)
19335:       warning ("off", "backtrace", "local");
19536:       warning ("Octave:savepath-local",
19536:                "savepath: current path saved to %s", file);
 7392:     endif
 7392:   end_unwind_protect


And changeset 19536 is


changeset:   19536:ffbd2cc7ca91
user:        John W. Eaton <jwe@octave.org>
date:        Mon Jan 05 12:15:51 2015 -0500
files:       scripts/path/savepath.m
description:
savepath: avoid possible error in struct concatenation

* savepath.m: Use cellfun to simplify extraction and concatenation of
struct elements.  Tag warning with ID and disable warning in test.


Rik <rik5>
Group administrator
Tue 09 Apr 2024 03:12:03 PM UTC, comment #17: 

At least on Windows, that message was helpful.
Especially on that platform, it is not entirely clear where Octave would create that file. Is it in the user folder (which is what I'd kind of expect from a "POSIX" app)? Is it in a subfolder of %LOCALAPPDATA% of that application? Is it in the (potentially remote) %APPDATA% folder of that application? Somewhere else?

It's not even clear what `~` should expand to on Windows.

With that message, it's pretty clear where to look for that file...

Markus Mützel <mmuetzel>
Group administrator
Tue 09 Apr 2024 02:50:55 PM UTC, comment #16: 

The 'Set Path' item is on the Edit menu.  This is a different issue.  Try typing 'savepath<RETURN>' in the Command Window and you will get the same warning message.  I think the function is simply trying to inform the user that no file name was specified so it is using the default file "~/.octaverc".  However, this just an informational message and shouldn't be displayed as a warning.  In fact, I can't see any reason to display it at all.  The documentation describes what happens when savepath() is called without arguments so there isn't any reason to emit a warning when the function is behaving exactly as described.

Rik <rik5>
Group administrator
Tue 09 Apr 2024 12:55:03 PM UTC, comment #15: 

Thank you for testing.

I pushed that patch to the stable branch:
https://hg.savann ... /rev/31225b5d494d

Re comment #10: I don't know what the "Set Path" dialog is.
Could you please describe step by step how to reproduce that issue?

Markus Mützel <mmuetzel>
Group administrator
Tue 09 Apr 2024 08:41:34 AM UTC, comment #14: 

The new version of savepath.m is working. Thank you.

There is still the detail of the prompt which disappears when saving the path from the Set Path dialog, as already explained in comment #10.

Charles Praplan <charprap>
Mon 08 Apr 2024 05:33:03 PM UTC, comment #13: 

@Charles: I have uploaded the patched copy of savepath.m to this bug report.  Could you try the same sequence as before and place this version of savepath.m in your current working directory and then test savepath()?

If all goes well we will check it in and it will be a part of the 9.2.0 bug fix release.

(file #55928)

Rik <rik5>
Group administrator
Mon 08 Apr 2024 03:16:08 PM UTC, comment #12: 

Thank you for the pointer. And sorry for having broken that.

Taking a closer look at the new regular expression, it is possible that it resulted in just matching the regular expression ".*" when trying to determine which paths share a prefix to an installed package. That can happen if for any installed package either the "dir" or (more likely) the "archprefix" field is empty. The latter is the case if a package doesn't install any binaries.

I probably tested the original change while I had only packages that install binaries.

That attached patch should fix that issue.


(file #55927)

Markus Mützel <mmuetzel>
Group administrator
Sat 06 Apr 2024 04:49:39 PM UTC, comment #11: 

@mmuetzel: Can you review this bug in savepath?  The regression was introduced with this changeset.


changeset:   32185:85e411ccf7e4
user:        Markus Mützel <markus.muetzel@gmx.de>
date:        Tue Jul 11 18:19:52 2023 +0200
summary:     savepath.m: Generalize filter for folders belonging to packages.


I think it is probably due to regexp handling and the filesep character on Windows which can be `\` or `/`.

Rik <rik5>
Group administrator
Sat 06 Apr 2024 10:05:20 AM UTC, comment #10: 

Additionally when saving the path from the Set Path dialog, the following warning is displayed:

   warning: savepath: current path saved to ~\.octaverc

and the prompt only appears again after pressing enter !!!

When typing savepath('~\.octaverc') in the command window, everything is ok.


Charles Praplan <charprap>
Sat 06 Apr 2024 09:51:50 AM UTC, comment #9: 

I confirm that version 9.1 of savepath.m deletes all path entries in .octaverc and that the version of file 55924 seems to be ok.

Charles Praplan <charprap>
Sat 06 Apr 2024 12:42:01 AM UTC, comment #8: 

There was a change to the file savepath.m in the 9.1 development cycle.  I'm attaching the old version of savepath.m to this bug report.  Could someone download it and put it in their current working directory.  Then run 'which savepath' to make sure that this new version is being used.  Then try an addpath() command and savepath() command and see if it works.  I suspect it will which will narrow it down to the exact changeset which caused this issue.

(file #55924)

Rik <rik5>
Group administrator
Fri 05 Apr 2024 10:25:48 PM UTC, comment #7: 

I have the same problem in windows 7 running in virtual-box.
The lines between:
## Begin savepath auto-created section, do not edit
## End savepath auto-created section
in .octaverc becomes empty after saving the path

I tried to solve it by open octave version 8.4.0 and save the path there. Then I open version 9.1.0 and everything work fine until a make a new savepath in version 9.1.0 then addpath command between ## begin and ## end are gone.
 
.octaverc saved with 8.4.0
## Begin savepath auto-created section, do not edit
  addpath ('C:\Users\tobbe\Documents\Ansoft;C:\Program Files\GNU Octave\Octave-8.4.0\mingw64\lib\octave\packages\windows-1.6.4\x86_64-w64-mingw32-api-v58;C:\Program Files\GNU Octave\Octave-8.4.0\mingw64\share\octave\packages\windows-1.6.4', '-begin');
## End savepath auto-created section


Then I addpath ('C:\Users\tobbe\Documents\HFSS');savepath in 9.1.0

the result in .octaverc :
## Begin savepath auto-created section, do not edit
## End savepath auto-created section

Torbjorn <tobbe>
Fri 05 Apr 2024 08:26:33 PM UTC, comment #6: 

So far I don't know who can check. I may find someone later.

By the way, I am running an installed Octave, namely:
"C:\Program Files\GNU Octave\Octave-9.1.0\octave-launch.exe" --gui
and it does not work.

Dimitry <dchuprakov>
Fri 05 Apr 2024 08:22:13 PM UTC, comment #5: 

Can someone else with a Windows platform check whether this might be an issue related to Windows?

Torsten Lilge <ttl>
Group Member
Fri 05 Apr 2024 08:18:29 PM UTC, comment #4: 

I just realized that savepath does not work whenever octave was started via run-octave. But it works for me in both versions (10 and 9) if it is started as installed program.

Torsten Lilge <ttl>
Group Member
Fri 05 Apr 2024 08:04:48 PM UTC, comment #3: 

Correct. My version of Octave is 9.1.0. It does not save the added paths to .ocatverc.

I may need to upgrade to 10.0.0 version. Thank you for the note.

Dimitry <dchuprakov>
Fri 05 Apr 2024 07:10:30 PM UTC, comment #2: 

Oh wait, version 10.0.0 works for me, but version 9.1.1 does not. Here, savepath only adds

## Begin savepath auto-created section, do not edit
## End savepath auto-created section

to .octaverc but misses to write the lines with the addpath() functions with the load path's additional directories.

Torsten Lilge <ttl>
Group Member
Fri 05 Apr 2024 07:01:46 PM UTC, comment #1: 

Thanks for the report. The "Save" button of the "Set Path" dialog works for me as expected, i.e. the manually added directories are written to my .octaverc file. Does the command "savepath" in the command window write the added paths to your .octaverc file?

Torsten Lilge <ttl>
Group Member
Fri 05 Apr 2024 11:01:49 AM UTC, original submission:  

Octave does not save folders added to path to .octaverc.
When I add new folders to path, they are properly added to the top of the Set Path window's list.
Then, when I press Save button in the Set Path dialog, the new added fodlers must be saved to .octaverc, but they are not.
As a result, when I close the Octave and restart it, those folders which I have added to path are not saved.
For the time being, I have to copy the folders added to path manually to .octaverc file.

Dimitry <dchuprakov>

 

Attached Files

Attached Files
file #55928:  savepath.m added by rik5 (9.1KiB - text/x-objcsrc)
file #55924:  savepath.m added by rik5 (8.3KiB - text/x-objcsrc)

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

Attach Files:
   
   
Comment:
   

 

Dependencies

This item does not depend on any other items.

No items depend on this one.

 

Mail Notification Carbon-Copy List

Carbon-Copy List
  • -email is unavailable- added by rik5
  • -email is unavailable- added by mmuetzel (Updated the item)
  • -email is unavailable- added by rik5
  • -email is unavailable- added by charprap (Posted a comment)
  • -email is unavailable- added by tobbe (Posted a comment)
  • -email is unavailable- added by rik5 (Updated the item)
  • -email is unavailable- added by ttl (Posted a comment)
  • -email is unavailable- added by dchuprakov (Submitted the item)
  •  

    Votes

    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.

     

    History

    Follow 17 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-04-12 rik5 StatusNeed Info Fixed
        Open/ClosedOpen Closed
        Fixed ReleaseNone 9.2.0
    2024-04-09 rik5 Carbon-Copy- Added jwe
    2024-04-09 mmuetzel StatusPatch Submitted Need Info
    2024-04-08 rik5 Attached File- Added savepath.m, #55928
    2024-04-08 mmuetzel Attached File- Added bug65559-savepath-empty-package-dir.patch, #55927
        StatusConfirmed Patch Submitted
        Operating SystemMicrosoft Windows Any
    2024-04-06 rik5 Item GroupIncorrect Result Regression
        StatusNone Confirmed
        Planned ReleaseNone 9.2.0
        SummaryOctave does not save folders added to path to .octaverc. savepath() regression no longer saves paths
        Carbon-Copy- Added mmuetzel
    2024-04-06 rik5 Attached File- Added savepath.m, #55924
    2024-04-05 rik5 CategoryNone Octave Function
        Item GroupPerformance Incorrect Result

    Back to the top

    Powered by Savane 3.16.
    Corresponding source code