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

 
 

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

Submitter:  Carnë Draug <carandraug>
Submitted:  Mon 21 Mar 2016 02:49:48 AM UTC
   
 
Category:  Octave Package Severity:  2 - Minor
Priority:  5 - Normal Item Group:  Other
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Closed
Release:  * dev Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 06 May 2020 08:01:31 PM UTC, comment #33: 

Spawned bug #58312 about postponing Java detetion. Let's continue discussion there.

Christian Häggström <saturn>
Wed 06 May 2020 07:54:22 PM UTC, comment #32: 

Bug #55174 doesn't really cover it, though, if the bug you are reporting is that loading the io package should not automatically call Java functions, and should be deferred until a function is called (which I tend to agree with).

Mike Miller <mtmiller>
Group Member
Wed 06 May 2020 07:51:47 PM UTC, comment #31: 

Thanks Philip. Seems it was already reported at https://savannah.gnu.org/bugs/?55174 . Sorry for the noise and for not searching existing bugs first!

Christian Häggström <saturn>
Wed 06 May 2020 07:43:33 PM UTC, comment #30: 

Please don't hijack a closed bug report, rather open a new one.
Once you've done so I'll comment on this issue there.

Thanks.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 06 May 2020 07:21:34 PM UTC, comment #29: 

The change http://hg.code.sf.net/p/octave/io/rev/2f5dfec4f128 causes octave to spam the console at module load time.

$ octave-cli --eval 'pkg load statistics'
OpenJDK 64-Bit Server VM warning: Archived non-system classes are disabled because the java.system.class.loader property is specified (value = "org.octave.OctClassLoader"). To use archived non-system classes, this property must be not be set

The spamming only occurs when default-jre-headless installed (Ubuntu 20.04)

It is better to postpone the detection of java until an octave function is called, and not have any side-effects at module load time.

Please guide me whether to post another bug about it here for octave-io, and/or a bug to octave-core to silence javachk("jvm").

Christian Häggström <saturn>
Wed 16 Oct 2019 07:12:27 PM UTC, comment #28: 

Pushed one more fix to properly test for 32- or 64-bit indexing Octave (thanks to Markus Muetzel for the suggestion), see:
http://hg.code.sf.net/p/octave/io/rev/503da59cb256

Philip Nienhuis <philipnienhuis>
Group Member
Fri 11 Oct 2019 08:22:40 PM UTC, comment #27: 

Sure, __have_feature__() can change at any moment but what would the real chance be in practice? That was my question.
But agreed, it's better to avoid the __ prepended functions, it's one less concern in the future.

Closing this report (although I'll push the last fixes later this weekend).

Philip Nienhuis <philipnienhuis>
Group Member
Fri 11 Oct 2019 07:51:17 PM UTC, comment #26: 

Then I probably didn't understand your question about the stability of the '__have_feature__' function. I don't think there is any need for stability, because it exists for Octave's own m-file functions to use, and can change at any time.

Am I right in understanding that this bug can be closed fixed?

Mike Miller <mtmiller>
Group Member
Fri 11 Oct 2019 07:45:49 PM UTC, comment #25: 

No need Mike, I already adapted the two functions where __have_feature__ () is called. Just need to push the changes.

It's just that (as I wrote in comment #22) one of those functions needs different code paths so it's a bit more code; that construct is temporary until io no longer depends on Octave < 6.0.0. To preprocess it in configure may be possible but would be pure overkill IMO.
__have_feature__ ("JAVA") is beautifully compact in comparison.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 11 Oct 2019 04:50:42 PM UTC, comment #24: 

IMHO all double underscore functions should be avoided as much as possible in code that is outside of Octave itself.

If it provides a feature that you need, we should discuss how we can meet that need on the maintainers list.

For example, I don't think you need to use "__have_feature__ JAVA" any longer with Octave 6, because the javachk function checks that for you already and reports an appropriate error message. Can we improve that interface somehow for you?

Mike Miller <mtmiller>
Group Member
Fri 11 Oct 2019 01:47:52 PM UTC, comment #23: 

OK, I just removed the last remaining occurrences of _octave_config_info_ (in _init_io_.m).
No different code paths needed, but io will now depend on Octave >= 4.0.0 (5 years old, backwards-compatible enough IMO).

On a related note:
How stable will the related "__have-feature__ ()" function be?
That is also used in some places in io.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 11 Oct 2019 12:13:02 PM UTC, comment #22: 

Now that bug #55429 has been fixed the io package can eliminate / replace the _octave_config_info_ JAVA check.
That is, for io to work with Octave-6.0.0+.  For older Octave versions io still has to rely on the _octave_config_info_ stuff.  So different code paths are required  for Octave-6.x.x and newer, and Octave-5.x.x and older. No big deal but it doesn't get easier nor does the code get smaller.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 09 Jan 2019 07:32:44 PM UTC, comment #21: 


> But I don't see how that can be done w/o some built-in function


No one is suggesting to do away with internal built in functions completely. The point is to expose the documented interfaces that are useful for you to use.

Obviously 'which.m' doesn't work without the '__which__' built in function. But you are not supposed to call '__which__' directly, its interface is not documented and not expected to be stable across multiple versions of Octave. You are obviously able to call it, but that doesn't mean you should call it in code that you expect to work across different Octave environments. That is the distinction that this bug report is about. To stop using internal functions that you shouldn't be using and identify what replacements, if any, that Octave needs to add to its public interface to fill in the gaps.

Mike Miller <mtmiller>
Group Member
Wed 09 Jan 2019 07:16:26 PM UTC, comment #20: 

If you look into the code of usejava.m and javachk.m you'll see they more or less have the very same basic code constructs.

Anyway, I think it could be a good solution if javachk.m could be improved to check for built-in Java support, if only for its name.
But I don't see how that can be done w/o some built-in function, and if such a built-in becomes available (can be called from n .m-file) it can also be called directly.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 09 Jan 2019 07:15:51 PM UTC, comment #19: 

Yeah, the point that Carnë made and I agree with is that 'javachk' can become that function that acts as a central diagnostic point for users and packages to query Octave whether Java support is enabled, whether a JVM can be started, and further whether various JRE features are present.

See bug #55429 for improving 'javachk', maybe in time for Octave 6, and in the meantime the io package can continue relying on '__have_feature__' and '__octave_config_info__' internal functions with the understanding that they are internal and not guaranteed to be stable from version to version now.

Mike Miller <mtmiller>
Group Member
Wed 09 Jan 2019 07:01:42 PM UTC, comment #18: 

Philip wrote:

> No, you're wrong, javachk.m (that I also wrote) does the same thing as usejava.m


But it doesn't have to do the same thing. Which is why Mike just said he will open a separate bug about changing it so it works for your case too.

Carnë Draug <carandraug>
Group Member
Wed 09 Jan 2019 06:56:25 PM UTC, comment #17: 

> This already exists. The function javachk ...

No, you're wrong, javachk.m (that I also wrote) does the same thing as usejava.m, i.e. only look if Octave succeeds in invoking a JRE.
It does nothing to assess whether Octave was built with Java support or not.

What is needed is a (low-level?) function that can, along the lines that __octave-config_info__ does.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 09 Jan 2019 06:10:00 PM UTC, comment #16: 


> This already exists. The function javachk


Aha, thank you. I will open a bug about improving that function, and add as a blocker for this bug.

Mike Miller <mtmiller>
Group Member
Wed 09 Jan 2019 06:01:05 PM UTC, comment #15: 

Mike Miller wrote:

> [...]  what if Octave itself provided a documented Java diagnostic function that could report what is wrong with loading Java? [...]


This already exists. The function javachk is meant to return a error message to help with such debugging. So I guess the error message could be adjusted to cover the two specific cases.

Carnë Draug <carandraug>
Group Member
Wed 09 Jan 2019 05:48:01 PM UTC, comment #14: 

There is no documented way at the moment for a user to ask Octave if it was supposed to be built with Java support, whether or not the Java runtime is found.

I can understand why this might be useful to help a user debug their installation, it tells them whether they need to install Octave again or just need to set JAVA_HOME or install a different JRE or something.

If this is specific to Java, because it relies on both compile-time features and run-time environment, what if Octave itself provided a documented Java diagnostic function that could report what is wrong with loading Java? This could take some of the work off of the io package and provide uniform messages for all packages and user code to rely on. If you agree we can move that to a separate wishlist bug.

Mike Miller <mtmiller>
Group Member
Wed 09 Jan 2019 07:40:56 AM UTC, comment #13: 

Just one more thing:
How can one find out if Octave was built with Java support? I'm asking because there's also a diagnostic function in io (chk_spreadsheet_support.m) that checks for several problems and/or potential problems with spreadsheet I/O. One of its steps is checking whether Octave has built-in Java support; if yes, it next checks for a working JRE.
usejava.m is no alternative because it combines these two steps into one.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 08 Jan 2019 06:59:20 PM UTC, comment #12: 

This is more about making sure that code outside of Octave does not rely on '__octave_config_info__'. There are no plans to remove that function, but we want to make sure no external code is depending on it so its contents can be changed at will without worrying about backwards compatibility. And if there are things that packages or users do need, then we want to add that to Octave in a documented manner.

So far I don't think we need to add anything to Octave to give you a path to migrate away from octave_config_info in the io package.

> Maybe it's still be a good place, so is there an alternative for octave_config_info("libdir")? It doesn't hurt to search there for .jars


No, there is no such equivalent. However, it is equal to


d = fullfile (OCTAVE_HOME, "lib");


if you want to keep searching that directory.

Mike Miller <mtmiller>
Group Member
Tue 08 Jan 2019 06:48:14 PM UTC, comment #11: 

Mike,

Reminiscences are slowly coming back, sorry for that. It's code I wrote years ago, that has always worked; originally it was even in PKG_ADD.

usejava
That's a good alternative, thanks. IIRC I wrote (large parts of) usejava.m.
usejava('jvm') checks if Java works at all in Octave, rather than just if Java support was built-in. The former is what is needed.
I know of other places in the io package code where this would be a better solution.

libdir
Ah, now I understand what you meant with "...jar files will be dropped into the Octave library directory on Windows" (comment #7). At the time from before mxe-octave, file hierarchy in mingw was less rigid, having Java .jars live in /lib was not so strange and even convenient.
Maybe it's still be a good place, so is there an alternative for octave_config_info("libdir")? It doesn't hurt to search there for .jars

ENABLE_64
uname().machine would do as well.
Fortran indexing bit width doesn't matter here, it's just that the bit widths (arch) of Octave, Java and LibreOffice should all match.

Thanks for the suggestions.
I suppose the code needs to be amended or octave-6.+? I expect current io-2.4.12 to be still working with upcoming Octave-5.1.0.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 07 Jan 2019 07:44:31 PM UTC, comment #10: 

No rush, I am not waiting for an immediate response, just replying to your points now as I am still fresh on what I looked at in the io package.

> ('build_features').JAVA - needed to know if Java support was built into Octave


You can use usejava("jvm") to detect whether Java support was built into Octave and whether the Java runtime can be loaded at the same time. Does checking this alone break something on Windows? If so, that can be reported separately and fixed in Octave. That is the whole point of the usejava function, so if it doesn't work for your needs, that's a bug.

> libdir - where are libraries situated. Not so much for MingW but also for some Linux distros; maybe sometime in the future for Octave as well if ever some spreadsheet I/O would move to core


Setting the value of libdir to octave_config_info("libdir") is only done in a 'if (ispc)' section of code, so this is absolutely Windows specific. So this is part of the logic I don't understand, is the package intentionally looking for jar files that the user has copied into Octave's lib directory? Is that documented somewhere as the recommended way to make Java libraries available to Octave? If that's not true, then there is no need to search libdir on Windows.

> ENABLE_64 - ( IIRC) needed to avoid 64-bit Octave find & try to invoke 32-bit LO as that might crash Octave. The other way round was more robust IIRC. I once had 32-bit and 64-bit Octave and 32-bit and 64-bit LO or OOo installations on the same box (admittedly a weird but not impossible scenario), IIRC on Linux and Windows.


As I asked before, do you really need to check ENABLE_64 or can you simply rely on the processor name returned from 'computer' or 'uname'? It seems to me that ENABLE_64 is irrelevant, only the computer architecture matters. Or are you saying that it is possible to have Octave compiled for a 64-bit processor that can still load 32-bit LibreOffice libraries, but only if it's not using 64-bit indexing? Remember that 64-bit indexing is not the same as 64-bit instruction set.

Mike Miller <mtmiller>
Group Member
Mon 07 Jan 2019 10:58:20 AM UTC, comment #9: 

I had a brief look in _init_io_.m and yes it's a tad more complicated than I initially remembered. It's slowly coming back :-)

The following calls to __octave_config_info__   exist in __init__io.m :

  • ('build_features').JAVA - needed to know if Java support was built into Octave


  • libdir - where are libraries situated. Not so much for MingW but also for some Linux distros; maybe sometime in the future for Octave as well if ever some spreadsheet I/O would move to core


  • ENABLE_64 - ( IIRC) needed to avoid 64-bit Octave find & try to invoke 32-bit LO as that might crash Octave. The other way round was more robust IIRC. I once had 32-bit and 64-bit Octave and 32-bit and 64-bit LO or OOo installations on the same box (admittedly a weird but not impossible scenario), IIRC on Linux and Windows.


Later this week I'll come back to it, I really have to do Real work now.

Philip Nienhuis <philipnienhuis>
Group Member
Mon 07 Jan 2019 07:29:51 AM UTC, comment #8: 

@Mike:
Thanks for reminding me, I forgot about this one.
I have little time until later this week; but reading TL;DR trough the dscussion here I think it's about automatically finding the place where users or distros keep .jar files.
That was meant as a service for users; in the past a lot of io package users had quite bit of trouble finding and loading the required Java libraries (.jars) for their favorite spreadsheet support SW, so I automated a lot of it based on existing code that checked the requirements for spreadsheet I/O.

A patch would be kind but I can also fix it myself if I know how to find out if there's built-in Java support w/o _octave_config_info_. Once there the existing io package code can do the rest, maybe after some adaptations a.o., for backwards compatibility considerations.
As to 32 vs. 64 bit Octave: that will get moot in the coming years I suppose. But I need to read up on what is at issue here.


Some background on why that code is there at all:
w/o Java, Octave + io can only read spreadsheet files in .xlsx (reasonably rapid), .ods (terribly slow) and .gnumeric (fairly quick but rarely used) formats.
Java support is indispensable for the older but still widely used .xls format and for acceptable speed of .ods I/O.
With the windows package, Octave can read all kinds of spreadsheet file formats, and rapidly, because then io invokes Excel behind the scenes and all formats that Excel can read are available (save .csv and .txt). Similar holds for LibreOffice (invoked through Java/UNO bridge). Note that -like Excel- LibreOffice can also be invoked through ActiveX/COM on Windows systems (I have code for that lying around but never finished it).

I don't expect Java jars to be dropped into Octave subdirs (is that what you mean?); all that is needed is finding where they are and load them (add to Java path). That is equally valid for Windows, Linux and OSX systems.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 06 Jan 2019 10:14:36 PM UTC, comment #7: 

Philip - it would be helpful if you could explain the logic that you need in the io package and what Octave could provide.

I don't really understand the differences with all of the possibilities of Java and LibreOffice 32/64 bit support on Windows that you are selecting. Can you explain why the value of computer("arch") or uname().machine is not sufficient? That should help eliminate the need for "ENABLE_64".

You should be able to eliminate all uses of octave_config_info("build_features").JAVA and have_feature("JAVA") immediately, I can provide a patch if you want.

I guess you expect that jar files will be dropped into the Octave library directory on Windows? Or is that an old assumption that can be dropped?

Mike Miller <mtmiller>
Group Member
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>
Group Member
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>
Group 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>
Group Member
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>
Group Member
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>
Group 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>
Group Member
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:


usejava ("jvm")
sizemax () > intmax ("int32")


Carnë Draug <carandraug>
Group 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)

 

Digest:
   bug dependencies.

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by saturn (Posted a comment)
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by carandraug (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-11 philipnienhuis StatusPostponed Fixed
        Open/ClosedOpen Closed
    2019-01-09 mtmiller Dependencies- Depends on bugs #55429
    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.13-d3ae.
    Corresponding source code