flac dev - Jan 2017 - [PATCH 0/5] Allow multiple targets to be disabled

Hi,

This patchet allows a few targets to be disabled when unrequired.
The rational is coming from VLC's contrib buildsystem, so far we use make -C
to select only some subparts of the available targets.
It would be easier and cleaner to use autoconf to do so IMHO.

There's an additional patch which fixes the build when building for
WinRT/UWP platform, upstreamed from VLC.
We have a couple more for this platform, but ultimately, I think the proper
solution would be to allow only libflac to be built, as most forbidden function
calls are in the convenience libraries in flac/shared.
I tried to switch those from noinst_LTLIBRARIES to EXTRA_LTLIBRARIES but failed
miserably at doing so :( I'm thinking it would be way easier to achieve by
moving to a non-recursive
automake configuration, but that's kind of an intrusive change; that being
said, I'm willing to do it.

Please let me know what you think!

Regards,

Hugo Beauzée-Luyssen (5):
  configure.ac: Allow the programs to be disabled
  configure.ac: Allow bench to be disabled
  configure.ac: Don't build any tests when they are explicitely disabled
  configure.ac: Allow examples to be disabled
  win_utf8_io: Avoid forbidden functions when building for WinRT/UWP

 configure.ac                        | 10 ++++++++++
 examples/Makefile.am                |  2 ++
 microbench/Makefile.am              |  4 ++++
 src/Makefile.am                     | 10 +++++++---
 src/flac/Makefile.am                | 13 +++++++++----
 src/metaflac/Makefile.am            |  8 ++++++--
 src/share/win_utf8_io/win_utf8_io.c |  9 +++++++++
 src/test_libFLAC++/Makefile.am      |  5 +++++
 8 files changed, 52 insertions(+), 9 deletions(-)

-- 
2.11.0

In case one only wants the libflac to be built
---
 configure.ac             |  3 +++
 src/flac/Makefile.am     | 13 +++++++++----
 src/metaflac/Makefile.am |  8 ++++++--
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/configure.ac b/configure.ac
index 070ab357..94449510 100644
--- a/configure.ac
+++ b/configure.ac
@@ -359,6 +359,9 @@ AC_DEFINE_UNQUOTED([FLAC__HAS_OGG],$FLAC__HAS_OGG,[define if
you have the ogg li
 AC_SUBST(FLAC__HAS_OGG)
 AC_SUBST(OGG_PACKAGE)
 
+AC_ARG_ENABLE([bin], AC_HELP_STRING([--disable-bin], [Do not build the flac
program]))
+AM_CONDITIONAL([FLaC__HAS_BIN], [test "${enable_bin}" !=
"no"])
+
 dnl check for i18n(internationalization); these are from libiconv/gettext
 AM_ICONV
 AM_LANGINFO_CODESET
diff --git a/src/flac/Makefile.am b/src/flac/Makefile.am
index bf3bf468..a6de246d 100644
--- a/src/flac/Makefile.am
+++ b/src/flac/Makefile.am
@@ -16,10 +16,6 @@
 #  with this program; if not, write to the Free Software Foundation, Inc.,
 #  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-bin_PROGRAMS = flac
-
-AM_CFLAGS = @OGG_CFLAGS@
-AM_CPPFLAGS = -I$(top_builddir) -I$(srcdir)/include -I$(top_srcdir)/include
 EXTRA_DIST = \
 	Makefile.lite \
 	Makefile.lite.iffscan \
@@ -31,6 +27,13 @@ EXTRA_DIST = \
 	iffscan.vcxproj \
 	iffscan.vcxproj.filters
 
+if FLaC__HAS_BIN
+
+bin_PROGRAMS = flac
+
+AM_CFLAGS = @OGG_CFLAGS@
+AM_CPPFLAGS = -I$(top_builddir) -I$(srcdir)/include -I$(top_srcdir)/include
+
 flac_SOURCES = \
 	analyze.c \
 	decode.c \
@@ -64,3 +67,5 @@ flac_LDADD = \
 	-lm
 
 CLEANFILES = flac.exe
+
+endif
diff --git a/src/metaflac/Makefile.am b/src/metaflac/Makefile.am
index 7a4ec6b6..5913139b 100644
--- a/src/metaflac/Makefile.am
+++ b/src/metaflac/Makefile.am
@@ -16,8 +16,6 @@
 #  with this program; if not, write to the Free Software Foundation, Inc.,
 #  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 
-bin_PROGRAMS = metaflac
-
 AM_CFLAGS = @OGG_CFLAGS@
 AM_CPPFLAGS = -I$(top_builddir) -I$(srcdir)/include -I$(top_srcdir)/include
 EXTRA_DIST = \
@@ -26,6 +24,10 @@ EXTRA_DIST = \
 	metaflac.vcxproj \
 	metaflac.vcxproj.filters
 
+if FLaC__HAS_BIN
+
+bin_PROGRAMS = metaflac
+
 metaflac_SOURCES = \
 	main.c \
 	operations.c \
@@ -58,3 +60,5 @@ metaflac_LDADD = \
 	@LIBICONV@
 
 CLEANFILES = metaflac.exe
+
+endif
-- 
2.11.0

---
 configure.ac           | 3 +++
 microbench/Makefile.am | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/configure.ac b/configure.ac
index 94449510..bdeba93a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -362,6 +362,9 @@ AC_SUBST(OGG_PACKAGE)
 AC_ARG_ENABLE([bin], AC_HELP_STRING([--disable-bin], [Do not build the flac
program]))
 AM_CONDITIONAL([FLaC__HAS_BIN], [test "${enable_bin}" !=
"no"])
 
+AC_ARG_ENABLE([benchmark], AC_HELP_STRING([--disable-benchmark], [Do not build
the flac benchmark]))
+AM_CONDITIONAL([FLaC__HAS_BENCHMARK], [test "${enable_benchmark}" !=
"no"])
+
 dnl check for i18n(internationalization); these are from libiconv/gettext
 AM_ICONV
 AM_LANGINFO_CODESET
diff --git a/microbench/Makefile.am b/microbench/Makefile.am
index 9ab6a040..9c960691 100644
--- a/microbench/Makefile.am
+++ b/microbench/Makefile.am
@@ -33,8 +33,12 @@ AM_CPPFLAGS = -I$(top_srcdir)/include
-I$(top_srcdir)/src/libFLAC/include
 
 noinst_HEADERS = util.h
 
+if FLaC__HAS_BENCHMARK
+
 noinst_PROGRAMS = benchmark_residual
 
 benchmark_residual_SOURCES = benchmark_residual.c util.c
 
 benchmark_residual_LDADD = @LIB_CLOCK_GETTIME@
+
+endif
-- 
2.11.0

---
 configure.ac                   |  1 +
 src/Makefile.am                | 10 +++++++---
 src/test_libFLAC++/Makefile.am |  5 +++++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/configure.ac b/configure.ac
index bdeba93a..9c852157 100644
--- a/configure.ac
+++ b/configure.ac
@@ -270,6 +270,7 @@ else
 FLAC__TEST_LEVEL=2
 fi
 AC_SUBST(FLAC__TEST_LEVEL)
+AM_CONDITIONAL(FLaC__HAS_TESTS, [test FLAC__TEST_LEVEL -gt 0])
 
 AC_ARG_ENABLE(werror,
 	AC_HELP_STRING([--enable-werror], [Enable -Werror in all Makefiles]))
diff --git a/src/Makefile.am b/src/Makefile.am
index 01417362..0e2ed05a 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -30,13 +30,17 @@ SUBDIRS = \
 	flac \
 	metaflac \
 	$(XMMS_DIRS) \
+	utils \
+	$(CPPLIBS_DIRS)
+
+if FLaC__HAS_TESTS
+SUBDIRS += \
 	test_grabbag \
 	test_libs_common \
 	test_libFLAC \
 	test_seeking \
-	test_streams \
-	utils \
-	$(CPPLIBS_DIRS)
+	test_streams
+endif
 
 EXTRA_DIST = \
 	Makefile.lite
diff --git a/src/test_libFLAC++/Makefile.am b/src/test_libFLAC++/Makefile.am
index fec7e81a..2d9b0016 100644
--- a/src/test_libFLAC++/Makefile.am
+++ b/src/test_libFLAC++/Makefile.am
@@ -23,6 +23,9 @@ EXTRA_DIST = \
 	test_libFLAC++.vcxproj.filters
 
 AM_CPPFLAGS = -I$(top_builddir) -I$(srcdir)/include -I$(top_srcdir)/include
+
+if FLaC__HAS_TESTS
+
 noinst_PROGRAMS = test_libFLAC++
 test_libFLAC___LDADD = \
 	$(top_builddir)/src/share/grabbag/libgrabbag.la \
@@ -44,4 +47,6 @@ test_libFLAC___SOURCES = \
 	encoders.h \
 	metadata.h
 
+endif
+
 CLEANFILES = test_libFLAC++.exe
-- 
2.11.0

---
 configure.ac         | 3 +++
 examples/Makefile.am | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/configure.ac b/configure.ac
index 9c852157..57cdcd4d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -366,6 +366,9 @@ AM_CONDITIONAL([FLaC__HAS_BIN], [test
"${enable_bin}" != "no"])
 AC_ARG_ENABLE([benchmark], AC_HELP_STRING([--disable-benchmark], [Do not build
the flac benchmark]))
 AM_CONDITIONAL([FLaC__HAS_BENCHMARK], [test "${enable_benchmark}" !=
"no"])
 
+AC_ARG_ENABLE([examples], AC_HELP_STRING([--disable-examples], [Do not build
the code examples]))
+AM_CONDITIONAL([FLaC__HAS_EXAMPLES], [test "${enable_examples}" !=
"no"])
+
 dnl check for i18n(internationalization); these are from libiconv/gettext
 AM_ICONV
 AM_LANGINFO_CODESET
diff --git a/examples/Makefile.am b/examples/Makefile.am
index efad1616..24744f26 100644
--- a/examples/Makefile.am
+++ b/examples/Makefile.am
@@ -20,7 +20,9 @@ if FLaC__WITH_CPPLIBS
 CPPLIBS_DIRS = cpp
 endif
 
+if FLaC__HAS_EXAMPLES
 SUBDIRS = c $(CPPLIBS_DIRS)
+endif
 
 EXTRA_DIST = \
 	Makefile.lite \
-- 
2.11.0

---
 src/share/win_utf8_io/win_utf8_io.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/src/share/win_utf8_io/win_utf8_io.c
b/src/share/win_utf8_io/win_utf8_io.c
index c61d27f3..1437b41e 100644
--- a/src/share/win_utf8_io/win_utf8_io.c
+++ b/src/share/win_utf8_io/win_utf8_io.c
@@ -34,6 +34,7 @@
 #endif
 
 #include <windows.h>
+#include <winapifamily.h>
 #include "share/win_utf8_io.h"
 #include "share/windows_unicode_filenames.h"
 
@@ -164,11 +165,13 @@ size_t strlen_utf8(const char *str)
 int win_get_console_width(void)
 {
 	int width = 80;
+#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
 	CONSOLE_SCREEN_BUFFER_INFO csbi;
 	HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
 	if(hOut != INVALID_HANDLE_VALUE && hOut != NULL)
 		if (GetConsoleScreenBufferInfo(hOut, &csbi) != 0)
 			width = csbi.dwSize.X;
+#endif
 	return width;
 }
 
@@ -176,6 +179,7 @@ int win_get_console_width(void)
 
 static int wprint_console(FILE *stream, const wchar_t *text, size_t len)
 {
+#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
 	DWORD out;
 	int ret;
 
@@ -202,6 +206,11 @@ static int wprint_console(FILE *stream, const wchar_t
*text, size_t len)
 	if (ret < 0)
 		return ret;
 	return len;
+#else
+    (void)stream;
+    OutputDebugStringW(text);
+    return len;
+#endif
 }
 
 int printf_utf8(const char *format, ...)
-- 
2.11.0

Hugo Beauzée-Luyssen <hugo at beauzee.fr> wrote:
> ---
>  src/share/win_utf8_io/win_utf8_io.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/src/share/win_utf8_io/win_utf8_io.c  
> b/src/share/win_utf8_io/win_utf8_io.c
> index c61d27f3..1437b41e 100644
> --- a/src/share/win_utf8_io/win_utf8_io.c
> +++ b/src/share/win_utf8_io/win_utf8_io.c
> @@ -34,6 +34,7 @@
>  #endif
> #include <windows.h>
> +#include <winapifamily.h>
According to  
<http://stackoverflow.com/questions/9509166/what-is-winapifamily-h>
winapifamily.h is only available since MSVS 2012, so build will fail in  
older
versions of Visual Studio (and in some older versions of MinGW).
>  #include "share/win_utf8_io.h"
>  #include "share/windows_unicode_filenames.h"
> @@ -164,11 +165,13 @@ size_t strlen_utf8(const char *str)
>  int win_get_console_width(void)
>  {
>  	int width = 80;
> +#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
Apparently these preprocessor defines are from winapifamily.h
so it won't work in older MSVS and MinGW.
>  	CONSOLE_SCREEN_BUFFER_INFO csbi;
>  	HANDLE hOut = GetStdHandle(STD_OUTPUT_HANDLE);
>  	if(hOut != INVALID_HANDLE_VALUE && hOut != NULL)
>  		if (GetConsoleScreenBufferInfo(hOut, &csbi) != 0)
>  			width = csbi.dwSize.X;
> +#endif
>  	return width;
>  }
> @@ -176,6 +179,7 @@ int win_get_console_width(void)
> static int wprint_console(FILE *stream, const wchar_t *text, size_t len)
>  {
> +#if WINAPI_FAMILY_PARTITION(WINAPI_PARTITION_DESKTOP)
Again, doesn't work with older MSVS and MinGW.
>  	DWORD out;
>  	int ret;
> @@ -202,6 +206,11 @@ static int wprint_console(FILE *stream, const  
> wchar_t *text, size_t len)
>  	if (ret < 0)
>  		return ret;
>  	return len;
> +#else
> +    (void)stream;
> +    OutputDebugStringW(text);
> +    return len;
> +#endif
>  }
Why OutputDebugStringW? MSDN says in  
<https://msdn.microsoft.com/en-us/library/windows/desktop/aa363362(v=vs.85).aspx>:
"Sends a string to the debugger for display. [...] If the application
has no debugger, the system debugger displays the string if the filter
mask allows it. [...] If the application has no debugger and the system
debugger is not active, OutputDebugString does nothing. [...] Applications
should send very minimal debug output and provide a way for the user
to enable or disable its use."
...it doesn't look like a function that simply prints some output from a  
program.


And what's the goal of this patch? To create flac.exe/metaflac.exe that
can work in WinRT? Or to remove build errors, and flac/metaflac are allowed
to work incorrectly?

On 01/06/2017 01:53 PM, Hugo Beauzée-Luyssen wrote:
> Hi,
> 
> This patchet allows a few targets to be disabled when unrequired.
> The rational is coming from VLC's contrib buildsystem, so far we use
make -C to select only some subparts of the available targets.
> It would be easier and cleaner to use autoconf to do so IMHO.
> 
> There's an additional patch which fixes the build when building for
WinRT/UWP platform, upstreamed from VLC.
> We have a couple more for this platform, but ultimately, I think the proper
solution would be to allow only libflac to be built, as most forbidden function
calls are in the convenience libraries in flac/shared.
> I tried to switch those from noinst_LTLIBRARIES to EXTRA_LTLIBRARIES but
failed miserably at doing so :( I'm thinking it would be way easier to
achieve by moving to a non-recursive
> automake configuration, but that's kind of an intrusive change; that
being said, I'm willing to do it.
> 
> Please let me know what you think!
> 
> Regards,
> 
> Hugo Beauzée-Luyssen (5):
>   configure.ac: Allow the programs to be disabled
>   configure.ac: Allow bench to be disabled
>   configure.ac: Don't build any tests when they are explicitely
disabled
>   configure.ac: Allow examples to be disabled
>   win_utf8_io: Avoid forbidden functions when building for WinRT/UWP
> 
>  configure.ac                        | 10 ++++++++++
>  examples/Makefile.am                |  2 ++
>  microbench/Makefile.am              |  4 ++++
>  src/Makefile.am                     | 10 +++++++---
>  src/flac/Makefile.am                | 13 +++++++++----
>  src/metaflac/Makefile.am            |  8 ++++++--
>  src/share/win_utf8_io/win_utf8_io.c |  9 +++++++++
>  src/test_libFLAC++/Makefile.am      |  5 +++++
>  8 files changed, 52 insertions(+), 9 deletions(-)
> 
Ping :)

-- 
Hugo Beauzée-Luyssen

Hugo Beauzée-Luyssen wrote:
>>   configure.ac: Allow the programs to be disabled
>>   configure.ac: Allow bench to be disabled
>>   configure.ac: Don't build any tests when they are explicitely
disabled
>>   configure.ac: Allow examples to be disabled
>>   win_utf8_io: Avoid forbidden functions when building for WinRT/UWP
>>
>>  configure.ac                        | 10 ++++++++++
>>  examples/Makefile.am                |  2 ++
>>  microbench/Makefile.am              |  4 ++++
>>  src/Makefile.am                     | 10 +++++++---
>>  src/flac/Makefile.am                | 13 +++++++++----
>>  src/metaflac/Makefile.am            |  8 ++++++--
>>  src/share/win_utf8_io/win_utf8_io.c |  9 +++++++++
>>  src/test_libFLAC++/Makefile.am      |  5 +++++
>>  8 files changed, 52 insertions(+), 9 deletions(-)
>>
>
> Ping :)
I think that FLAC sources should remain compatible with Visual
Studio 2010 (and probably earlier), people still need it.
For example: <https://github.com/xiph/flac/issues/23>

Is it possible to simply exclude win_utf8_io.c from compiling
if target OS is WinRT/non-desktop?