bugGNU Octave - Bugs: bug #47480, [octave forge] (io) relies on...

 
 

bug #47480: [octave forge] (io) relies on deprecated octave_config_info

Submitted by:  Carnë Draug <carandraug>
Submitted on:  Mon 21 Mar 2016 02:49:48 AM UTC  
 
Category:  Octave Forge Package Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Other
Status:  Postponed Assigned to:  Philip Nienhuis <philipnienhuis>
Originator Name:  Open/Closed:  Open
Release:  dev 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)

Wed 23 Mar 2016 09:27:29 PM UTC, comment #6:

I pushed your cset after some edits (function names, commit message) and pushed another fix for removing/simplifying several octave_config_info calls.

Adapting several other calls to octave_config_info will have to wait until further developments in core Octave.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Wed 23 Mar 2016 04:29:08 PM UTC, comment #5:

> Why does io_init.m (or enter_io_package.m) and io_exit.m need a texinfo header at all?


For the same reason all private functions in octave have a texinfo. They usually only have one line saying that it's a private undocumented function. If you don't have it, you will get a warning. Users will always know about the function. It's in their workspace afterall. Better to have help text reminding them it's private and undocumented than nothing at all.

> But I would like to avoid the texinfo show up in the generated doc-cache; no user should ever run it nor even know about it. So better no mention of it anywhere. Does the texinfo show up anywwhere in the io package's documentation?
> A mere plain text line with "undocumented" or so would do IMO.


Even if you have a single line of plain text, it will appear on the doc-cache file. If you don't want it to appear in the list of functions, simply omit it from the INDEX file.

> As to the GPL license text, fine.


You will need a license header in all files with code. That it was absent in the PKG_ADD file could be an issue. Please remove my name from it if that's the issue.

Carnë Draug <carandraug>
Project Member
Wed 23 Mar 2016 07:44:53 AM UTC, comment #4:

@Carnë

I'll morph your and my patches into one and push later.

A question:

Why does _io_init_.m (or _enter_io_package_.m) and _io_exit_.m need a texinfo header at all?

As to the GPL license text, fine.

But I would like to avoid the texinfo show up in the generated doc-cache; no user should ever run it nor even know about it. So better no mention of it anywhere. Does the texinfo show up anywwhere in the io package's documentation?
A mere plain text line with "undocumented" or so would do IMO.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 22 Mar 2016 11:14:51 PM UTC, comment #3:

Apologies Carnë, I didn't note you had supplied an attachment with the first post.
The bug title is misleading, I figured it was about octave_config_info, but your patch is really about PKG_ADD.

Tonight, in spite of what I wrote earlier, I already locally fixed:

- PKG_ADD/PKG_DEL issues (using _io_init_.m and _io_exit_.m with PKG_ directives), they work fine.) I didn't know it was so easy. post_install.m, PKG_ADD and PKG_DEL are gone.
(I think a link to the wiki (http://wiki.octave.org/Creating_packages) on the developers page on Octave-Forge wouldn't hurt; I just stumbled on that wiki page by accident. The discussion between you and me went so far because I never knew about the exact way pkg.m works and how it installs / initializes / loads packages. Your bug report on bug #47481 supplied the final piece I was missing all along.)

- Many octave_config_info occurrences, also in the _io_init_ / _io_exit_ files.
The JAVA ones are all done, the ENABLE_64 / USE_64_BIT_IDX_T ones not as the latter form (for older Octaves) just don't work with _have_feature_().
I haven't built 64-bit indexing Octave recently so I'm not sure if ENABLE_64 works reliably.

- libdir is more enigmatic. On *nix it is usually /usr/bin, on Windows [OCTAVE_HOME filesep "lib"], but I consider this not very robust. And I don't know what it is in cygwin and on Mac OSX.
That is the very reason octave_config_info is so sweet - it just returns the required info without "if windows elseif osx elseif cygwin else <*nix>" constructs on the user side.
OTOH libdir is used in io just to search Java .jar files; there's no guarantee they'll be in the libdir or subdirs. Some Linux distros keep them there, but Mac OSX or cygwin? And on Windows it is especially uncertain. I even disabled it for *nix after a bug report about delays but in fact that was more because searching the OpenOffice .jars took so ridiculously long.

My stuff is already in my local repo, I'll test it tomorrow on Linux and if I don't hit issues I'll push.

Hopefully that eases your mind :-)

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Tue 22 Mar 2016 03:35:40 AM UTC, comment #2:

Since it doesn't influence the io package operation, and since it fixes a real issue for those running development versions, what improvements should I made to the patch before pushing it?

This patch is not just merely cosmetic. It also removes the risk of clearing variables from the user workspace during the loading of the io package (as explained on the first post). Also, pushing it now will mean that the next version of the io package will still be installable in future versions of Octave (or if it doesn't, at least won't be because of this issue).

Carnë Draug <carandraug>
Project Member
Mon 21 Mar 2016 10:07:51 PM UTC, comment #1:

Confirmed & postponed.

This bug report is only valid for the current & future state of the core Octave development version. It doesn't influence the io package's operation at all, not even until Octave 4.3+. It's merely cosmetic, therefore priority => minor.

As soon as a new stable 4.2 (or will it be 5.0?) Octave release is in sight I'll finally adapt those parts of the code that can do without octave_config_info; but that is not valid for all calls.
Til then we will have to live with a warning about a deprecated function upon loading the io package.

Philip Nienhuis <philipnienhuis>
Project MemberIn charge of this item.
Mon 21 Mar 2016 02:49:48 AM UTC, original submission:

The io package relies on octave_config_info for a couple of things:

0. second guess pkg on where the oct files will be installed
0. check for java support
0. something else in _UNO_spsh_open_ that checks if it was built with 64 bit indexing

The attached patch fixes the first issue. On top of not using octave_config_info, it has the two other advantages of: 1) not moving files around during installation in a way that is way too dependent on knowing the internals of pkg; 2) does not add and clear variables from the user workspace.

The two others can be trivially fixed with:

Carnë Draug <carandraug>
Project Member

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #36704:  fix_io_hack.cset added by carandraug (14KiB - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by carandraug (Submitted the item)
  •  

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

    Date Changed by Updated Field Previous Value => Replaced by
    2017-08-13 jwe Summaryio package relies on deprecated octave_config_info => [octave forge] (io) relies on deprecated octave_config_info
    2016-03-21 philipnienhuis Severity3 - Normal => 2 - Minor
        StatusNone => Postponed
        Assigned toNone => philipnienhuis
        Releaseother => dev
    2016-03-21 carandraug Attached File- => Added fix_io_hack.cset, #36704

    Back to the top


    Powered by Savane 3.3