How to fix the path to the mexfiles in package biosig

Hello,

I'm trying to build a new version of biosig4octave [2]. The package
seems to build and install fine,
except that the mex-functions from the biosig/src/ directory are not
included when doing
pkg load biosig

mexSLOAD.mex is build and available in
...\octave\biosig-2.3.1\x86_64-w64-mingw32-api-v56\mexSLOAD.mex
but the directory is not included in the search path.

The Biosig package for Octave is available here and can be installed with
pkg install
https://pub.ist.ac.at/~schloegl/biosig/prereleases/biosig4octave-2.3.1.src.tar.gz

In order to test this you can run
pkg load biosig
which sload % works ok
which mexSLOAD % not available in the path, although it is build on

I've read the documentation [1], specifically the section about
package/INDEX, package/DESCRIPTION and categories, but
it did not help me understanding how to fix this. Can you provide a hint
what is needed in the package definition ?

Cheers,
Alois

[1] Creating Packages (GNU Octave)
[2] BioSig

This works for me.
However, I need to restart Octave after installing the package.

I noticed that I cannot completely unload the package with pkg unload biosig. The package is still marked as loaded in pkg list. But the corresponding paths were still removed from the load path.
pkg doesn’t load a package if it is already marked as loaded. That might be the reason that requires the restart.

I’m not sure what is causing this weird unload behavior.
There are two peculiarities about the biosig package I noticed. But I haven’t looked in detail if they cause this behavior:

  1. The PKG_ADD file adds some paths to the Octave load path. But there is no PKG_DEL file that would remove them from the load path when the package is unloaded.
  2. The PKG_ADD file contains commands to load other packages. This might mess up the logic of the pkg function. I’m not sure if that function correctly handles being called recursively this way. Consider adding those packages to the Depends in the package DESCRIPTION file instead if it depends on those packages.

@Markus:
Did you issue a "clear -f’ after uninstalling biosig?
At least on Windows, “pkg unload” manages to clear the package paths but cannot unload binary modules due to the relevant files being locked and so breaks at the point that the pkg subdir for binary modules is to be wiped. pkg.m silently exits at that point, or maybe complains but with a deceiving error message.
Those binary modules only get locked when invoked (or one of the subfunctions inside is invoked), which a.o., happens when the documentation for a package is created.
Several years ago Mike Miller assisted to investigate this but progress stalled, see GNU Octave - Bugs: bug #45864, mkoctfile is unable to overwrite... [Savannah] and GNU Octave - Bugs: bug #55143, pkg install only succeeds on... [Savannah].

As to PKG_AD / PKG_DEL, Carnë convinced me to have separate package initialization and cleanup scripts and only add just one line to call those scripts in PKG_ADD and PKG_DEL. I forgot the exact reason but I do have some reminiscence that in hindsight Carnë was quite right. Anyway, see examples in the io package.

Also, pkg.m has been patched some time ago to also automatically unload package dependers if a package gets unloaded, also including checks for dependencies on/from other packages. So I’d agree with Markus’ suggestion to better move dependency info to the DESCRIPTION file.

@PhilipN: I didn’t uninstall the biosig package. Did you have problems doing that?

I only tried to install the package. Tried to load and unload the package repeatedly (with and without restarting Octave).
Like I wrote, I didn’t look into it in any detail. So, I don’t understand what you are trying to say.
Are you suggesting that pkg install should run a clear -f at some point?

You did just an install of biosig, I did an uninstall followed by install.

As to “clear -f”:
I think “pkg unload”, “pkg uninstall” and “pkg update” should also unload the package’s binary modules from memory; on Windows that is vital because loaded module files remain locked as long as the modules remain in memory. On Linux I’m less sure if this is needed.
If uninstalling & updating packages implicitly include "unload’ (I suppose so) it could probably be inserted somewhere in the unload scripts.

The following mainly pertains to Windows:
As outlined in bug #45864 (not about “pkg.m” but about mkoctfile, but the underlying issue is exactly the same), it isn’t as easy as it looks. “clear -f” would also unload binary modules from other packages (it’s kind of unscrupulous), so there needs to be some selectivity. Reading back in bug #45864 I infer that it’s more or less known what is needed (for mkoctfile) but it never got finished.
Clearing individual functions is surely possible using the “clear” function, but what’s needed for pkg.m is to find out which binary modules there are in a package; just the file names of the binary modules is insufficient as a module (actually a .dll) can have many entry points (other binary functions in the same .oct file). I don’t know if modules with multiple entry points will be unloaded if the main, or just any of thebinary functions in the.oct file, are "clear"ed.

Even then I’m not sure if this is the issue Alois hit; but at first sight it looks (looked) like issues reported earlier and repeatedly in the help ML and some bug reports.
My question to you was if you did a “clear -f”; well, you didn’t. So it now looks like Alois hit another issue.

Thanks for the response and clarification. This was certainly useful in
order to get a better understanding of the issue.
I can confirm that after a restart of Octave and "pkg load biosig", the
path to mexSLOAD is included.

Currently, I do not have PKG_ADD and PKG_DEL, but an inst/install.m is
provided. It seems that PKG_ADD is derived from this.

I've tried a number of things to fix this, including your suggestion to
move some of these weak dependencies into biosig/DESCRIPTION in the
field Depends:, and defining PKG_ADD myself. But none seems to improve
the situation. Since this seems to be a "known" issue, and the work
around exists(restart of Octave), I tend to release the next version of
biosig4octave as is.

Cheers,
Alois

P.S.: If you have any further suggestions how to improve the situation,
let me know.

I don’t know the release tools for your package. But I’d be surprised if they generated a PKG_ADD file automatically with that content.
That file is in your release tarball in the folder inst.
Even if it didn’t resolve the issue here, it would probably be good to add a PKG_DEL file that at least removes the corresponding folders from Octave’s load path. Otherwise users could be surprised by functions that might still shadow their own functions even after the package is unloaded…

@Markus;

Yes, you are right. There is a PKG_ADD in biosig4octave, the "pkg load
..." instructions are now removed and moved into
biosig/DESCRIPTION -> "Recommends:" .
Adding the PKG_DEL seems to address the issue. The updated package is
available here
https://pub.ist.ac.at/~schloegl/biosig/prereleases/biosig4octave-2.3.1.src.tar.gz

@Markus and PhilipN:
Thanks for all the hints.

Attached is also a patch for mxe-octave, to build libbiosig. The first
part of the patch upgrades to the latest version of libbiosig v2.3.1.

The second part contains some hints about building the package
"biosig4octave". Maybe this helps to distribute the package
"biosig(4octave)" as part of mxe-octave. What do you think ? Would that
be feasible ?

Cheers,
Alois

P.S.: If you have any further suggestions how to improve the situation,
let me know.

mxe-octave-upgrade-biosig-to-v231.diff (1.24 KB)

We already added build rules that install the biosig package on the default branch of MXE Octave. That was done last time you updated the package.
Those are in src/opkg-biosig.mk. (The opkg prefix stands for “Octave package”. There are other packages with the prefix of. These are hosted on Octave Forge.)
Should that be updated as well?

The current default branch will probably be used for Octave 7 which will be released probably some time next year.

The release branch is used for building bug fix releases (currently Octave 6.x). There will probably be at least one other bug fix release for Octave 6. If that should happen, it would be probably some time this year.
Currently the biosig Octave package is not included in the release branch. But the biosig library is. The corresponding build rules are in src/biosig.mk on that branch.
Should the biosig library be updated on the release branch, too?

Edit: I probably should have mentioned that there are “almost nightly” builds from the default and release branch of MXE Octave here: Buildbot (octave.space)

@Markus

thanks for the reminder and explanation of the different branches and
libbiosig, biosig, and opkg-biosig makefiles.
Attached is the patch for libbiosig and opkg-biosig.

Thanks also for the hint to the nightly builds available on the
buildbot, I was not aware of that.

I do suggest to include libbiosig and (opkg-)biosig in the
default-branch and in the release branch. The API between libbiosig and
the mex-functions of Biosig have not changed. Even if a user has Biosig
installed in the local environment, it should still work after an
upgrade of Octave (i.e. an upgrade of libbiosig from v2.2.1 to v2.3.1).
Therefore, I do not expect any detrimental effects by including
(opkg-)biosig in the current release branch, but it would provide the
additional functionality of Biosig to the users of the next release of
Octave.

Cheers,
Alois

upgrade-biosig-2.3.1.patch (1.22 KB)

There are currently no build rules to install Octave packages that are not hosted on Octave Forge on the release branch of MXE Octave. And I’d prefer to not backport those changes from the default branch to the release branch.
IIUC, we’d like to limit changes on the release branch to “minor” updates and bug fixes if possible.

I hope that it is ok with you to just update biosig.mk to the newer library version on the release branch (while we are on the Octave 6.x cycle). Users will still be able to install the biosig Octave package like before.

I’ve pushed a change to the release branch with the update here:
mxe-octave: ac813229ff2c

With that change, I can install the prerelease with pkg install https://pub.ist.ac.at/~schloegl/biosig/prereleases/biosig4octave-2.3.1.src.tar.gz. Loading and unloading the package now works without issues.
pkg test biosig stops early on (with encoding issues - maybe in help texts?). This might be a non-issue if you don’t have any Octave-compatible BISTs and the package works for you otherwise.
However, I don’t know how to use the package and can’t tell if it is working or not.

I’m currently trying to build with your updates on the default branch and will push those changes if they build correctly.

Building opkg-biosig failed for me after the update. The linker complains about many undefined symbols.
opkg-biosig (75.8 KB)

I tried to build Octave 7. So, this might also be caused by something in the development version of Octave.
Also, I tried to do an incremental build. Quite a few packages were updated since I last built. So, this could also be caused by some kind of version mix up.

I tried with downgrading to libbiosig and opkg-biosig version 2.2.1 in the same build tree and it compiled correctly. So, I’d guess the build tree is ok and it is some change in the “biosig” package that might be causing this.

@Alois: Do you have an idea what might have caused this kind of error from the package site?

I installed this patch to biosig4octave-2.3.1 locally and it compiled correctly:

Use $(MKOCTFILE) variable instead of hardcoded "mkoctfile"

$ diff -Naup ./src/Makefile.orig ./src/Makefile
--- ./src/Makefile.orig	2021-07-20 07:01:49.000000000 +0200
+++ ./src/Makefile	2021-07-26 09:59:22.601812600 +0200
@@ -200,10 +200,10 @@ endif
 
 ifneq (:,/usr/bin/octave)
 %.mex: %.cpp
-	mkoctfile $(DEFINES) -I.. -v -g --mex "$<" -L.. -lbiosig -o "$@"
+	$(MKOCTFILE) $(DEFINES) -I.. -v -g --mex "$<" -L.. -lbiosig -o "$@"
 
 %.oct: %.cpp
-	mkoctfile $(DEFINES) -I.. "$<" -L.. -lbiosig -o "$@"
+	$(MKOCTFILE) $(DEFINES) -I.. "$<" -L.. -lbiosig -o "$@"
 endif
 
 %.mexw32: %.cpp

Would you consider this change for your package?

Edit: Loading and unloading the bundled package seems to work correctly after that change.

Btw: I’m getting this shadowing warning when loading the package:

>> pkg load biosig
warning: function D:\SVN\Octave\test\octave-2021-07-26-10-03-w64 biosig\octave-2021-07-26-10-03-w64\mingw64\share\octave\packages\biosig-2.3.1\t300_FeatureExtraction\hurst.m shad
ows a core library function
warning: called from
    D:\SVN\Octave\test\octave-2021-07-26-10-03-w64 biosig\octave-2021-07-26-10-03-w64\mingw64\share\octave\packages\biosig-2.3.1\PKG_ADD at line 6 column 1
    load_packages_and_dependencies at line 56 column 5
    load_packages at line 53 column 3
    pkg at line 590 column 7

Is shadowing that core function intentional?