patchGNU Octave - Patches: patch #10058, src/biosig.mk: update to v2.2.1

 
 

patch #10058: src/biosig.mk: update to v2.2.1

Submitter:  Alois Schlögl <schloegl>
Submitted:  Wed 14 Apr 2021 08:34:30 PM UTC
   
 
Category:  Core : other Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 25 Apr 2021 04:57:42 PM UTC, comment #13: 

The buildbot configurations we have seem to be happy now.
But given the amount of necessary follow-up changes, it is probably ok if this stays on the default branch for now.

Closing report as done.

Markus Mützel <mmuetzel>
Group administrator
Fri 23 Apr 2021 06:15:38 PM UTC, comment #12: 

It looks like there were still issues if MXE Octave used the compiler installed on the build system.
That should hopefully be fixed with this change:
https://hg.octave.org/mxe-octave/rev/3960e6022e88


Markus Mützel <mmuetzel>
Group administrator
Thu 22 Apr 2021 05:07:09 PM UTC, comment #11: 

I pushed some patches that should allow to build some dependencies of libbiosig with MXE Octave natively.
With that, I pushed the following change that overhauls the build rule of libbiosig (allowing it to build natively on GNU/Linux):
https://hg.octave.org/mxe-octave/rev/399c7b225e13

I had to patch the Makefile to allow installation for (cross) mingw.
I'm attaching that patch here. Maybe you'd like to do something similar upstream.

(file #51304)

Markus Mützel <mmuetzel>
Group administrator
Thu 22 Apr 2021 02:14:20 PM UTC, comment #10: 

The attached patch allows me to complete a native GNU/Linux build successfully.

I haven't checked yet if it would break a cross-build for Windows.

(file #51303)

Markus Mützel <mmuetzel>
Group administrator
Thu 22 Apr 2021 07:27:40 AM UTC, comment #9: 

The last version of libbiosig fails to compile for GNU/Linux (native) with the following error:

x86_64-pc-linux-gnu-ar rcs libbiosig.a biosig.o biosig2.o crc4scp.o sopen_cfs_read.o sopen_heka_read.o sopen_igor.o sopen_scp_read.o sopen_abf_read.o sopen_alpha_read.o sopen_axg_read.o sopen_scp_write.o sopen_hl7aecg.o gdftime.o mdc_ecg_codes.o physicalunits.o sandbox.o xgethostname.o sopen_famos_read.o Address.o MetricMeasure.o UnitCode.o HealthCareProviderId.o INT-U64.o INT-I64.o INT-U32.o INT-I32.o INT-U16.o INT-I16.o INT-U8.o INT-I8.o BITS-32.o BITS-16.o BITS-8.o FEFString.o FEFFloat.o Fraction.o AbsoluteRange.o Handle.o HandleRef.o DeviceCode.o UnitsOfMeasurementCode.o MetricsCode.o BodySiteCode.o AlertCode.o ExtNomenclatureCode.o ParameterGroupCode.o PrivateCode.o Placeholder.o ManufacturerID.o AbsoluteTime.o RelativeTime.o HighResRelativeTime.o ObservationTime.o SimpleColour.o ExtNomenRef.o PersonNameGroup.o PersonName.o OID-Type.o SessionArchiveSection.o ArchiveProtection.o ManufacturerSpecificSection.o ManufacturerSpecificEncoded.o ManufacturerSpecificBinary.o HealthCareProviderSection.o HealthCareProfessionalSection.o Authorization.o PatientDemographicsSection.o PatMeasure.o PatientSex.o PatientType.o PatientRace.o SessionTestSection.o MedicalDeviceSystemSection.o SystemModel.o SystemSpecEntry.o ProdSpecEntryType.o ProdSpecEntry.o ApplicationArea.o PowerStatus.o LineFrequency.o MedicalDeviceComponents.o VirtualMedicalDeviceSection.o MsmtPrinciple.o ChannelDescriptionSection.o ChannelStatus.o MultimediaSection.o Comments.o ContentOrReference.o SessionPhaseSection.o DescriptiveDataSection.o MetricDescriptiveDataSection.o MetricStatus.o MetricCategory.o MetricCalEntry.o MetricCalType.o MetricCalState.o MeasurementStatus.o SampleArrayDescriptiveDataSection.o SaSpec.o StorageDataType.o SaFlags.o ScaleRangeSpec.o SaFilterEntry.o FilterType.o SaGridEntry.o SaCalData.o SaCalDataType.o SaSignalFrequency.o RealTimeSampleArrayDescriptiveDataSection.o TimeSampleArrayDescriptiveDataSection.o DistributionSampleArrayDescriptiveDataSection.o DsaRangeSpec.o NumericDescriptiveDataSection.o EnumerationDescriptiveDataSection.o MeasuredDataSection.o SampleArrayMeasuredDataSection.o SampleArrayMeasuredDataBlock.o RealTimeSampleArrayMeasuredDataSection.o TimeSampleArrayMeasuredDataSection.o MarkerEntryRelTim.o DistributionSampleArrayMeasuredDataSection.o NumericMeasuredDataSection.o ChoiceOfNuObsValue.o NuObsValue.o DispResolution.o EnumerationMeasuredDataSection.o ChoiceOfEnumObsValue.o EnumObsValue.o EnumVal.o EnumObject.o EnumRecordMetric.o EnumRecordOO.o ChoiceOfEnumTimeStamp.o AlertSection.o AlertCondition.o AlertControls.o AlertFlags.o AlertType.o LimitSpecEntry.o CurLimAlStat.o DynamicAttributeChangeSection.o SessionNotesSection.o ANY.o GeneralizedTime.o INTEGER.o NativeEnumerated.o NativeInteger.o OBJECT_IDENTIFIER.o REAL.o UTF8String.o asn_SEQUENCE_OF.o asn_SET_OF.o constr_CHOICE.o constr_SEQUENCE.o constr_SEQUENCE_OF.o constr_SET_OF.o OCTET_STRING.o BIT_STRING.o asn_codecs_prim.o ber_tlv_length.o ber_tlv_tag.o ber_decoder.o constr_TYPE.o constraints.o der_encoder.o xer_support.o xer_decoder.o xer_encoder.o per_opentype.o per_support.o per_decoder.o per_encoder.o sopen_fef_read.o sopen_rhd2000_read.o scp-decode.o
make[3]: x86_64-pc-linux-gnu-ar: No such file or directory
make[3]: *** [Makefile:725: libbiosig.a] Error 127


Afaict, the native `ar` executable isn't prefixed in MXE.

Markus Mützel <mmuetzel>
Group administrator
Tue 20 Apr 2021 07:07:18 PM UTC, comment #8: 

Thank you.

I added a build rule for this Octave package to MXE Octave here:
https://hg.octave.org/mxe-octave/rev/b532d40c63d8

With that, the biosig package is listed with the installed packages and I am able to load it.
I added the demo subfolder and ran `demo2` (after replacing one occurrence of `unix` with `system`).
It looks like something happened (a figure popped up and I was asked for input). So, I guess it is working.

After that, I renamed the "biosig" package to "libbiosig" to make it clearer what it actually builds. (Also "libbiosig" is used as a requirement in the DESCRIPTION file of the "biosig" Octave package.)
https://hg.octave.org/mxe-octave/rev/4e99aee38edb

Since these patches required changes to the main Makefile, I thought it would be best to push them to the default branch.
At the same time, I don't think they are very risky.
Maybe we can wait a few days for the results from the buildbots. After that, we can still decide whether we'd like to graft them to the release branch.

Marking as ready for test.

Markus Mützel <mmuetzel>
Group administrator
Sun 18 Apr 2021 04:31:47 PM UTC, comment #7: 


Ok, lets give a try. Here is a first (experimental) version of the package "Biosig for Octave"

  pkg install https://pub.ist.ac.at/~schloegl/biosig/prereleases/biosig4octave-2.2.1.src.tar.gz

  pkg load biosig

I do not want to call this mexbiosig, because I consider this experimental.
Let me know of any issues that you see.



Alois Schlögl <schloegl>
Sun 18 Apr 2021 09:39:11 AM UTC, comment #6: 

I pushed the part that updates to the newer version of the library to the release branch here:
https://hg.octave.org/mxe-octave/rev/fa5ce462f09d

Additionally, I prepared a patch that adds rules to build and install "mexbiosig" with the Windows installer (see attachment).

I noticed that the tarball only contains the files for four .mex files.
IIUC, the biosig package for Matlab/Octave also consists of several .m files. (Basically, the files in the "biosig4matlab" folder in the complete tarball.)
It might be more convenient for Octave users of your package if the package tarball also contained those .m files. Otherwise, only the .mex files would be installed.
Would you consider including those in the "inst" folder of the package tarball?
If you do, it might also make sense to add a PKG_ADD file that calls initialization commands when the package is loaded. IIUC, that could be mostly identical to your install.m script.
See also:
https://octave.org/doc/v6.2.0/Creating-Packages.html


(file #51286)

Markus Mützel <mmuetzel>
Group administrator
Sat 17 Apr 2021 10:00:26 AM UTC, comment #5: 


As discussed, please go forward. An updated version of mexbiosig is available here:

https://pub.ist.ac.at/~schloegl/biosig/prereleases/mexbiosig-2.2.1.src.tar.gz

Please check whether this would work.


Alois Schlögl <schloegl>
Fri 16 Apr 2021 12:56:24 PM UTC, comment #4: 

Thanks for explaining. That probably means that it is good to go on the release branch. There is probably a low risk of breaking anything apart from the biosig4octmat package anyway.

Wrt installing the package itself, I see the following rules for Octave (Forge) packages in `Makefile.in` in the MXE Octave repository:

ifeq ($(MXE_SYSTEM),msvc)
define OCTAVE_FORGE_PKG_BUILD
    $(INSTALL) -d '$(3)$(HOST_PREFIX)/local/octave-$(octave_VERSION)/share/octave'
    $(INSTALL) -d '$(3)$(HOST_PREFIX)/local/octave-$(octave_VERSION)/lib/octave'
    cd '$(1)/..' && tar cvfz $($(PKG)_FILE) $($(PKG)_SUBDIR)
    cd '$(1)/..' && $(HOST_PREFIX)/local/octave-$(octave_VERSION)/bin/octave-cli.exe -H -f -q \
        --eval "more off;" \
        --eval "pkg('prefix', '`cd "$(3)" && pwd -W`$(HOST_PREFIX)/local/octave-$(octave_VERSION)/share/octave/packages', '`cd "$(3)" && pwd -W`$(HOST_PREFIX)/local/octave-$(octave_VERSION)/lib/octave/packages');" \
        --eval "pkg('install', '-verbose', '$($(PKG)_FILE)');"
endef
else
define OCTAVE_FORGE_PKG_BUILD
    if [ $(ENABLE_BINARY_PACKAGES) = no ]; then \
      mkdir -p '$(3)$(HOST_PREFIX)/src'; \
      cd '$(1)/..' \
        && tar cvfz $($(PKG)_FILE) $($(PKG)_SUBDIR) \
        && $(INSTALL) -m644 '$($(PKG)_FILE)' '$(3)$(HOST_PREFIX)/src'; \
    else \
      cd '$(1)/..' && \
      tar cfz $($(PKG)_FILE) $($(PKG)_SUBDIR) && \
        OCTAVE_CONFIG=$(MXE_OCTAVECONFIG) TMP="$(1)/.." \
        $(CONFIGURE_CPPFLAGS) $(CONFIGURE_LDFLAGS) \
        MKOCTFILE=$(MXE_MKOCTFILE) MAKE=$(MAKE) \
        CONFIGURE_OPTIONS="$(HOST_AND_BUILD_CONFIGURE_OPTIONS)" $(4) \
        $(PYTHON3) $(TOP_DIR)/tools/pkg-install.py install -verbose "$($(PKG)_FILE)"; \
     fi
endef
endif


I'm not sure if MXE Octave can still be used with msvc currently. (At least, I don't know of anyone that currently uses it that way.) So it is probably ok to focus on the rule in the `else` branch for now.

IIUC, the (patched) tarballs of the packages are installed instead of the "actual" packages if ENABLE_BINARY_PACKAGES is "no".
I'm not sure if this will be important for your package. Anyway, I think that we can probably defer that to later.

The python script that the `else` branch is calling is this one:
https://hg.octave.org/mxe-octave/file/tip/tools/pkg-install.py

We'd probably need to do something equivalent with the biosig4octmat package if we want to get it installed correctly by MXE Octave. That's probably at least the later part of `install_pkg` and the entire(?) `rebuild_pkg` function.

"biosig4octmat" is a bit of an "oddball" in that it's packaging a library + an Octave package in the same tarball.

From an MXE Octave point of view, it would be much easier if we could split installation of the library and the package into two separate tarballs/packages.
In that case, we could use the former part (or the existing one) to install the library like we currently do.
Ideally, we could just use the OCTAVE_FORGE_PKG_BUILD rule to build and install the Octave package tarball.

I slightly modified the Makefile in your mexbiosig tarball and added a MXE Octave buildrule for it.
I don't know if that modification would break Matlab compatibility. But it looks like the package compiled and installed its files correctly in a test with MXE Octave for me.
See attached files.

Do you think, this is something you could work with?


(file #51279, file #51280)

Markus Mützel <mmuetzel>
Group administrator
Fri 16 Apr 2021 09:04:09 AM UTC, comment #3: 



The new release fixes a crash under certain circumstances, because of memory allocation errors. I consider this to have the potential of being security related. The patch to fix this is attached.


The new release (2.2.1) has also a number of other improvement, especially in "biosig4octmat", but I believe these are currently not installed by "biosig.mk". The corresponding lines are commented. The reason was that I'm not sure whether the directory layout would ok. Fell free to uncomment these, if you believe these are in a good enough shape.



(file #51275)

Alois Schlögl <schloegl>
Thu 15 Apr 2021 06:27:10 PM UTC, comment #2: 

The checksum didn't match.
I ran `make update-checksum-biosig` and got the following SHA-1 checksum: f16bfc7e0f05901e09283020d7cd565bed13c75e

With that, the package downloaded and compiled for me.

Markus Mützel <mmuetzel>
Group administrator
Thu 15 Apr 2021 04:35:00 PM UTC, comment #1: 

Thank you for the update.

We recently re-activated the release branch in MXE Octave.
IIUC, dot-releases of Octave (i.e. Octave 6.3) will be built from that branch. The next major release (i.e. Octave 7) will most likely be built from the default branch.

We haven't (yet) established a fixed set of rules for which updates should go to the release branch and which to the default branch.
Maybe as a rule of thumb: updates of Octave packages (and packages that enable Octave package updates) should probably go to the release branch. Updates that fix crashes or are otherwise important (like maybe security fixes?) should probably also go to the release branch.
Everything else should probably go to the default branch.

To estimate which category this update might fit in: Is this update required for an update of your Octave package "biosig4octmat"? Or does it fix crashes?

Markus Mützel <mmuetzel>
Group administrator
Wed 14 Apr 2021 08:34:30 PM UTC, original submission:  



the attached patch update biosig to v2.2.1


Alois Schlögl <schloegl>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #51304:  libbiosig-1-mingw-install.patch added by mmuetzel (2KiB - application/octet-stream)
file #51303:  mxe-libbiosig-native.patch added by mmuetzel (7KiB - application/octet-stream)
file #51286:  mxe-opkg-biosig.patch added by mmuetzel (6KiB - application/octet-stream)
file #51279:  mexbiosig-2.0.6.src.tar.gz added by mmuetzel (35KiB - application/x-gzip)
file #51280:  mexbiosig.mk added by mmuetzel (563B - text/plain)
file #51269:  update-biosig-v221.diff added by schloegl (627B - 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 schloegl (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 logged-in users can vote.

     

    Follow 11 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2021-04-25 mmuetzel StatusReady For Test Done
        Open/ClosedOpen Closed
    2021-04-22 mmuetzel Attached File- Added libbiosig-1-mingw-install.patch, #51304
    2021-04-22 mmuetzel Attached File- Added mxe-libbiosig-native.patch, #51303
    2021-04-20 mmuetzel StatusNeed Info Ready For Test
    2021-04-18 mmuetzel Attached File- Added mxe-opkg-biosig.patch, #51286
    2021-04-16 mmuetzel Attached File- Added mexbiosig-2.0.6.src.tar.gz, #51279
        Attached File- Added mexbiosig.mk, #51280
    2021-04-16 schloegl Attached File- Added fix-edf2gdf-and-json-export.patch, #51275
    2021-04-15 mmuetzel StatusNone Need Info
    2021-04-14 schloegl Attached File- Added update-biosig-v221.diff, #51269

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code