bugGNU Octave - Bugs: bug #54636, path is not be reinitialized if a...

 
 

bug #54636: path is not be reinitialized if a class directory is modified

Submitted by:  Adam Dodd <adamd1008>
Submitted on:  Sun 09 Sep 2018 04:30:55 PM UTC  
 
Category:  Interpreter Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Unexpected Error
Status:  Patch Submitted Assigned to:  None
Originator Name:  Adam Dodd Open/Closed:  Open
Release:  4.4.1 Operating System:  Any

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)

Thu 20 Sep 2018 09:06:00 PM UTC, comment #6:

Thanks Mike. Updated patch as advised. Apologies for the rusty start!

Re-ran the tests with the same result:

Manual tests (as described below) for bug #46282 and bug #54636 also pass.

I was rather phased by the test log not uploading. It's 5.3 MB and there was no error message when submitting it! I've compressed it this time (1.1 MB), hopefully that works.

Let me know if there's anything else.

(file #45068, file #45069)

Adam Dodd <adamd1008>
Thu 20 Sep 2018 06:37:23 PM UTC, comment #5:

Yes, probably a file size issue. The "upload size limit" mentioned is not reliable.

Thanks for working on improving this. The new patch looks good, better that it doesn't need to add a new method to the class.

The indentation on the nested 'if' statements in 'subdirs_modified' is not quite right. And you might as well combine all of those nested 'if's into one complex condition, since there is no 'else'. That might look something like

The first summary line of the commit message should mention this and any other bug reports fixed by this change with a "(bug #1, #2, #3)" tag at the end of the line.

Thanks for doing a full test suite run as well as showing the test cases that this bug resolves.

Mike Miller <mtmiller>
Project Administrator
Thu 20 Sep 2018 06:03:01 PM UTC, comment #4:

Trying to attach test log again (didn't work; size issue?)

Adam Dodd <adamd1008>
Thu 20 Sep 2018 05:50:29 PM UTC, comment #3:

Attached new patch:

  • subdirs_modified is now a static function (in namespace octave due to use of sys:: classes)

I ran `make check` for this patch, summary:

Full log attached. Also did some quick manual testing as outlined below, all good.

Please let me know if anything else needs changing.

(file #45064)

Adam Dodd <adamd1008>
Mon 17 Sep 2018 09:35:34 PM UTC, comment #2:

I have not yet tested this patch, but at a glance it looks like it is doing something sensible and the coding style looks mostly correct. I'm not sure that the new function 'subdirs_modified' needs to be a member function of the 'octave::load_path::dir_info' class, I think it could be a bare file scoped function.

Mike Miller <mtmiller>
Project Administrator
Sun 09 Sep 2018 04:34:14 PM UTC, comment #1:

Extra info: I asked about this issue some time ago at: https://stackoverflow.com/questions/49335462/how-to-refresh-octaves-knowledge-of-class-methods

Adam Dodd <adamd1008>
Sun 09 Sep 2018 04:30:55 PM UTC, original submission:

Octave has a bug where removal of a class function script file will cause a "no such file" error to be thrown.

Please note that the attached patch fixes this bug as well as bug #46282. It incorporates a fix to bug #46281 and #53856 committed in changeset 25374:c8f49ee7a687.

To reproduce, I used the current head of the Mercurial repo:

Do the following:

Now, using another terminal:

Returning to Octave terminal:

A similar approach can be taken to reproduce bug #46282. Foo/bar example taken from that bug.

The issue is that load_path::dir_info::update checks the top directory's timestamp (in this case /tmp/tmp.SxVxOnqAot), when it's the @myobj or +Foo folder which has its timestamp updated.

My patch fixes these:

In each case, I renamed the file for the second call and reinstated it for the third.

Adam Dodd <adamd1008>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #45068:  54636_r3.patch added by adamd1008 (3KiB - text/x-patch - r3: patch, test log)
file #45069:  fntests_r3.log.bz2 added by adamd1008 (1MiB - application/x-bzip - r3: patch, test log)
file #45064:  54636_r2.patch added by adamd1008 (3KiB - text/x-patch - New patch, test log)
file #44960:  mypatch.patch added by adamd1008 (4KiB - text/x-patch - Patch file)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by adamd1008 (Submitted the item)
  • -email is unavailable- added by adamd1008
  •  

    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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-09-20 adamd1008 Attached File- => Added 54636_r3.patch, #45068
        Attached File- => Added fntests_r3.log.bz2, #45069
    2018-09-20 adamd1008 Attached File- => Added 54636_r2.patch, #45064
    2018-09-19 mtmiller Carbon-CopyRemoved 80942 => -
    2018-09-10 mtmiller Carbon-CopyRemoved 80942 => -
    2018-09-10 mtmiller Item GroupOther => Unexpected Error
        StatusNone => Patch Submitted
    2018-09-09 adamd1008 Attached File- => Added mypatch.patch, #44960
        Carbon-Copy- => Added tpapastylianou

    Back to the top


    Powered by Savane 3.3