bugGNU Octave - Bugs: bug #57081, [octave forge] (io) incorrect io...

 
 

bug #57081: [octave forge] (io) incorrect io package version in src/configure.ac

Submitter:  Andrew Janke <apjanke>
Submitted:  Sat 19 Oct 2019 04:13:01 AM UTC
   
 
Category:  Octave Package Severity:  2 - Minor
Priority:  3 - Low Item Group:  Other
Status:  Fixed Assigned to:  philipnienhuis
Originator Name:  Open/Closed:  * Closed
Release:  * other Operating System:  * Any
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Fri 04 Sep 2020 08:06:11 PM UTC, comment #12: 

@Kai:
You didn't try with Octave-4.2.0, a pity as your patch mentions "is-numeric" but in 4.2.0- it used to be "is_numeric_type".
I fixed that in the repo.

Furthermore, io was already dependent on Octave >= 4.2.0.
I forgot but tried with Octave-3.6.4 and obviously that didn't work, not even with "-nodeps".
So I backed out the change for supporting even older Octave releases.

I've just tried with Octave-4.2.0 and 7.0.0 and it seems to work fine now.
Again thanks Kai !

Closing report.

Philip Nienhuis <philipnienhuis>
Group Member
Thu 03 Sep 2020 08:42:32 AM UTC, comment #11: 

@Markus: You are more than right with comment #10 😨 Thank you.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 03 Sep 2020 07:35:16 AM UTC, comment #10: 

The condition in comment #6 doesn't look correct to me. You probably want to have something like:

#if OCTAVE_MAJOR_VERSION < 4 || (OCTAVE_MAJOR_VERSION == 4 && OCTAVE_MINOR_VERSION < 4)


With the condition you propose in comment #6, version 3.8 wouldn't pass because 8 is not smaller than 4.

Markus Mützel <mmuetzel>
Group administrator
Thu 03 Sep 2020 07:13:51 AM UTC, comment #9: 

Too many negations, I wanted to say:

"Trying" to support older Octave versions is good.  Your package seems loosely coupled with Octave internals.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 03 Sep 2020 07:11:56 AM UTC, comment #8: 

Thank you Philip.  There is no doubt that Olaf's solution worked, I used it without knowing about it for about two years now ^^  But maintaining GNU Autotools is indeed tough.

Your change in comment #6 "OCTAVE_MAJOR_VERSION <= 4" looks good to me.  There is no reason "trying" to support older Octave versions, as your package seems not to be too tightly coupled with Octave internals.

Kai Torben Ohlhus <siko1056>
Group Member
Wed 02 Sep 2020 05:59:45 PM UTC, comment #7: 

Pushed here:
http://hg.code.sf.net/p/octave/io/rev/0d19d897c9b0

and setting status to "Ready for test".

One of these days I'll make new Linux and Windows build and I'll try an older Octave release. If other people would try please do.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 02 Sep 2020 03:42:34 PM UTC, comment #6: 

Thanks Kai, patch looks good, will try tonight.

At the time Olaf added the configure stuff I already had a hunch that it could have been easier, but I lacked time to investigate and most of all: it just worked. I'm still grateful anyway that Olaf made the change.

Shall / can I change

+#if OCTAVE_MAJOR_VERSION == 4 && OCTAVE_MINOR_VERSION < 4


into

+#if OCTAVE_MAJOR_VERSION <= 4 && OCTAVE_MINOR_VERSION < 4


to also support ancient versions like 3.8? I saw bug reports for that version not too long ago.
In several .m-files there are still constructs to allow them to work with even older Octave versions.
As long as there are no real blockers, and code changes are limited as regards coding itself plus performance, I have no issues with keeping up compatibility with quite ancient Octave releases.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 02 Sep 2020 09:22:06 AM UTC, comment #5: 

Dear Philip, I am a happy user of your io package for long time 🙂

Please find attached the desired patch to makes things hopefully easier again.

In general the idea is to make your package code Octave 4.4+ compatible and the header looks like this


#if ! defined (OCTAVE_4_2_COMPATIBILITY_H_)
#define OCTAVE_4_2_COMPATIBILITY_H_ 1

#include <octave/version.h>

#if OCTAVE_MAJOR_VERSION == 4 && OCTAVE_MINOR_VERSION < 4
  #define iscell    is_cell
  #define isempty   is_empty
  #define isnumeric is_numeric
#endif

#endif


The only affected Octave versions are 4.2 >= VER > 4.4.  Because the "iscell" -> "is_cell" change was made for Octave 4.4 and you claim to support the Octave 4.2 series, which I fully understand.

I tested my patch in Octave 5.2.0 and 4.4.1.  To my shame I must admit to not have an Octave 4.2 ready to go now.  But I can prepare one if necessary.

Once you drop support for Octave 4.2, just burn my header and the three includes of them 😅

Please let me know about the patch =)


(file #49733)

Kai Torben Ohlhus <siko1056>
Group Member
Wed 02 Sep 2020 06:19:44 AM UTC, comment #4: 

I'm

  1. unfamiliar with autoconf stuff
  2. unhappy with the bootstrap/autoconf etc. files, IIRC thay gave rise to other issues in the past

so in principle I'd agree.

But they currently are needed to select the proper form of is_numeric() or isnumeric() or the like in calls in a.o, csv2cell.cc.

Those calls changed just one or two Octave releases ago and the autoconf stuff is there to guarantee backward compatibility of the io psckage with older Octave versions.
Olaf -like me- wanted to support the io package in older Octave, as it is probably one of the most used packages and several other OF packages depend on it.

If you have a sample header for me I'd happily try it out, a new io release is planned soon.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 02 Sep 2020 12:56:22 AM UTC, comment #3: 

Looking at your Makefile

https://sourceforge.net/p/octave/io/ci/default/tree/src/Makefile

neither "bootstrap" nor "configure.ac" are necessary, as you only use "mkoctfile" to compile a few files.

To comply with the Octave package standard

https://octave.org/doc/v5.2.0/Creating-Packages.html

it is not necessary either, only the "Makefile" is called then.

If you want to support older Octave versions, you can include a small header defining "is_cell" to "iscell", etc., or just mark the latest supported Octave version in the DESCRIPTION file.

I cannot find an Octave Forge documentation which specifies the necessity for "bootstrap" nor "configure.ac".  Maybe Olaf can clarify this?

Kai Torben Ohlhus <siko1056>
Group Member
Tue 01 Sep 2020 05:34:45 PM UTC, comment #2: 

While I've been updating this line for new releases when I remembered to do so, my question about the necessity of that line is still unanswered ...

Philip Nienhuis <philipnienhuis>
Group Member
Sat 19 Oct 2019 11:49:09 AM UTC, comment #1: 

Thanks for spotting this.
I'm unfamiliar with autotools, Olaf added these configure/bootstrap things  for me when there was a name change in internal core Octave (is_cell -> iscell, etc.) that broke csv2cell and friends.

The fact that this particular stanza doesn't do a thing (as the io package just works with it) is an indication for me that it serves no purpose.
Can it be deleted?

Priority => low
Severity => Minor

Philip Nienhuis <philipnienhuis>
Group Member
Sat 19 Oct 2019 04:13:01 AM UTC, original submission:  

io 2.4.13 is out, and the version in DESCRIPTION is correct, but in src/configure.ac, it still says this:


AC_INIT([io], [2.4.10], [Philip Nienhuis <prnienhuis@users.sf.net>])


The attached patch fixes the version number there. But it will immediately be out of date, since work on 2.4.14/2.5.0 will be opened up soon.

Andrew Janke <apjanke>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #49733:  of-io-no-bootstrap.patch added by siko1056 (10KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Date Changed by Updated Field Previous Value => Replaced by
    2020-09-04 philipnienhuis StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2020-09-02 philipnienhuis StatusIn Progress Ready For Test
    2020-09-02 philipnienhuis StatusNeed Info In Progress
    2020-09-02 siko1056 Attached File- Added of-io-no-bootstrap.patch, #49733
    2020-09-01 jwe Summary[io] incorrect io package version in src/configure.ac [octave forge] (io) incorrect io package version in src/configure.ac
    2019-10-19 philipnienhuis Severity3 - Normal 2 - Minor
        Priority5 - Normal 3 - Low
        StatusNone Need Info
        Assigned toNone philipnienhuis
    2019-10-19 apjanke Attached File- Added octave-io-fix-package-version-in-configure_ac.patch, #47716

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code