package/gtkiostream: new package

This package provides many useful software engineering and signal processing includes and linkable library.

Signed-off-by Matt Flax <[email protected]>

---
  package/Config.in                  |  1 +
  package/gtkiostream/Config.in      | 36 ++++++++++++++++++++++++++++++
  package/gtkiostream/gtkiostream.mk | 21 +++++++++++++++++

  3 files changed, 58 insertions(+)

create mode 100644 package/gtkiostream/Config.in create mode 100644 package/gtkiostream/gtkiostream.mk

package/Config.in +1 -0

index befeadc27b..8e3dd0c08d 100644
@@ -1328,6 +1328,7 @@ menu "Audio/Sound"
  	source "package/bcg729/Config.in"
  	source "package/caps/Config.in"
  	source "package/fdk-aac/Config.in"
+	source "package/gtkiostream/Config.in"
  	source "package/libao/Config.in"
  	source "package/libasplib/Config.in"
  	source "package/libbroadvoice/Config.in"

package/gtkiostream/Config.in new file +0 -0

new file mode 100644
index 0000000000..9951e9d465

 Hi Matt,

Quoted 8 lines · Matt Flax

This package provides many useful software engineering and signal processing includes and linkable library.

Signed-off-by Matt Flax <[email protected]>

---
  package/Config.in                  |  1 + package/gtkiostream/Config.in      | 36 ++++++++++++++++++++++++++++++ package/gtkiostream/gtkiostream.mk | 21 +++++++++++++++++

 Please add yourself to the DEVELOPERS file [1] for this package. This way, you'll get an e-mail if the package fails in the autobuilders, or when a new version is released if the package is registered on release-monitoring.org.

 Also, you are missing a gtkiostream.hash with the tarball and license file hashes [2].

 When re-posting, please add a patch changelog below the --- line, and also add the version in the subject using the -v3 option to git format-patch. See [3].

Quoted 22 lines

  3 files changed, 58 insertions(+)

  create mode 100644 package/gtkiostream/Config.in create mode 100644 package/gtkiostream/gtkiostream.mk

package/Config.in +0 -0

index befeadc27b..8e3dd0c08d 100644
@@ -1328,6 +1328,7 @@ menu "Audio/Sound"

      source "package/bcg729/Config.in"
      source "package/caps/Config.in"
      source "package/fdk-aac/Config.in"
+    source "package/gtkiostream/Config.in"
      source "package/libao/Config.in"
      source "package/libasplib/Config.in"
      source "package/libbroadvoice/Config.in"

package/gtkiostream/Config.in new file +0 -0

new file mode 100644
index 0000000000..9951e9d465

package/gtkiostream/Config.in line 3

Quoted 3 lines

package/gtkiostream/Config.in new file +3 -0

@@ -0,0 +1,36 @@
+config BR2_PACKAGE_GTKIOSTREAM
+        bool "gtkiostream"
+  depends on BR2_TOOLCHAIN_HAS_OPENMP

 Indention is with a tab. Please use utils/check-package to verify the coding style.

 Also, does this package really require OpenMP?

package/gtkiostream/Config.in line 4

Quoted 1 line

package/gtkiostream/Config.in new file +1 -0

+  depends on BR2_USE_WCHAR # required by Flac

 Use lowercase flac, and no "required by":

    depends on BR2_USE_WCHAR # flac

Quoted 4 lines

package/gtkiostream/Config.in new file +3 -0

+  depends on BR2_INSTALL_LIBSTDCPP # required by Eigen
+    select BR2_PACKAGE_EIGEN
+  select BR2_PACKAGE_EIGEN_UNSUPPORTED_MODULES > +    select 

BR2_PACKAGE_FFTW

 Please keep the selects alphabetical.

Quoted 8 lines

+  select BR2_PACKAGE_FFTW_DOUBLE
+  select BR2_PACKAGE_FFTW_SINGLE
+    select BR2_PACKAGE_ALSA_LIB
+    select BR2_PACKAGE_LIBSAMPLERATE
+  select BR2_PACKAGE_ALSA_UTILS
+  select BR2_PACKAGE_ALSA_UTILS_AMIXER
+  select BR2_PACKAGE_ALSA_UTILS_APLAY
+  # The following are for sox

 So, all of those are optional sox features. Does gtkiostream really need all of them to function? If not, please don't select them.

Quoted 5 lines

+  select BR2_PACKAGE_FILE
+  select BR2_PACKAGE_FLAC
+  select BR2_PACKAGE_LAME
+  select BR2_PACKAGE_LIBMAD
+  select BR2_PACKAGE_LIBPNG

Quoted 8 lines

+  select BR2_PACKAGE_LIBSNDFILE
+  select BR2_PACKAGE_LIBVORBIS
+  select BR2_PACKAGE_OPENCORE_AMR
+  select BR2_PACKAGE_OPUSFILE
+  select BR2_PACKAGE_LIBID3TAG
+  select BR2_PACKAGE_LIBGSM
+  select BR2_PACKAGE_LIBAO
+  select BR2_PACKAGE_SOX

 I've taken a quick look at configure.ac, and it looks like sox is optional.

Quoted 2 lines

+        help
+          gtkiostream package

 Please provide an actual help text. What you put in the commit log is actually pretty good.

package/gtkiostream/Config.in line 35

Quoted 4 lines

package/gtkiostream/Config.in new file +4 -0

+
+          https://github.com/flatmax/gtkiostream
+
+comment "gtkiostream needs OpenMP"

 You also need to add a comment for all the other depends (wchar, C++). Look at other Config.in for examples.

Quoted 7 lines

+    depends on !BR2_TOOLCHAIN_HAS_OPENMP

package/gtkiostream/gtkiostream.mk new file +0 -0

new file mode 100644
index 0000000000..36a84e86c7

package/gtkiostream/gtkiostream.mk line 6

Quoted 8 lines

package/gtkiostream/gtkiostream.mk new file +6 -0

@@ -0,0 +1,21 @@
+################################################################################ 
 
+#
+# GTKIOSTREAM
+#
+################################################################################ 
 
+GTKIOSTREAM_VERSION = HEAD

 You must provide an actual version. This can be tag, or if there are no releases, a commit hash (all 40 characters). In the latter case, please explain in the commit log what this hash corresponds to (e.g. "latest commit as of ...").

Quoted 3 lines

+GTKIOSTREAM_SITE = $(call github,flatmax,gtkiostream,$(GTKIOSTREAM_VERSION)) +GTKIOSTREAM_LICENSE = GPLv2

 We use SPDX abbreviations, so GPL-2.0. Also, AFAICS all files have the "or later" clause, so it would be GPL-2.0+.

 Please also add a license file (and hash).

package/gtkiostream/gtkiostream.mk line 10

Quoted 2 lines

package/gtkiostream/gtkiostream.mk new file +2 -0

+GTKIOSTREAM_INSTALL_STAGING = YES
+GTKIOSTREAM_INSTALL_TARGET = YES

 This is the default so not needed.

Quoted 3 lines

package/gtkiostream/gtkiostream.mk new file +2 -0

+GTKIOSTREAM_CONF_OPTS = --disable-octave
+GTKIOSTREAM_DEPENDENCIES = host-pkgconf libsamplerate eigen 

fftw-double fftw-single alsa-utils sox

 Also alphabetical please.

 You select alsa-lib in Config.in but don't depend on it. Why do you select it?

 Since this line is getting a bit long, it's nicer to split it:

GTKIOSTREAM_DEPENDENCIES = \
    host-pkgconf \
    alsa-utils \
    ...

 I saw in configure.ac that in addition to sox, there are a lot of other optional packages. Since they can't be explicitly enabled/disabled, it chooses purely based on the presence of the pkg-config file if the optional package is enabled or not. That makes things non-reproducible, since depending on the order of the build, the feature will be included or not. Therefore, you need to track these dependencies:

    $(if $(BR2_PACKAGE_LIBGTK2),libgtk2) \
    $(if $(BR2_PACKAGE_LIBGTK3),libgtk3) \
    ...

package/gtkiostream/gtkiostream.mk line 14

Quoted 2 lines

package/gtkiostream/gtkiostream.mk new file +2 -0

+
+GTKIOSTREAM_AUTORECONF = YES

 Please add a comment above to explain why this is needed:

# Fetched from git, no configure script

package/gtkiostream/gtkiostream.mk line 18

Quoted 4 lines

package/gtkiostream/gtkiostream.mk new file +4 -0

+
+define GTKIOSTREAM_M4
+  pushd $(@D); ./tools/autotoolsClean.sh;    mkdir -p m4; popd
+endef

 Please indent with a tab inside the define.

 pushd/popd is not needed; just:

    cd $(@D); \
        ./tools/autotoolsClean.sh; \
        mkdir -p m4

 However, looking at the source, I doubt this pre-configure hook is needed at all:
- the m4 directory already exists (it contains emscripten.m4);
- autotoolsClean.sh doesn't do anything unless a build was already done (but in Buildroot, you'd do 'make gtkiostream-dirclean' to clean that up).

 I have marked your patch as Changes Requested in patchwork, so unless you resend it (with the fixes mentioned above), we will forget about it.

 Thank you!

 Regards,
 Arnout

[1] https://nightly.buildroot.org/#DEVELOPERS
[2] https://nightly.buildroot.org/#adding-packages-hash
[3] https://nightly.buildroot.org/#_patch_revision_changelog

package/gtkiostream/gtkiostream.mk line 21

Quoted 3 lines

package/gtkiostream/gtkiostream.mk new file +3 -0

+GTKIOSTREAM_PRE_CONFIGURE_HOOKS += GTKIOSTREAM_M4
+
+$(eval $(autotools-package))