Reimplement configure.ac

Motion-Project / motion

Motion, a software motion detector. Home page: https://motion-project.github.io/

GNU General Public License v2.0

3.64k stars 547 forks source link

Reimplement configure.ac #955

Closed vilhelmgray closed 3 years ago

vilhelmgray commented 5 years ago

The configure.ac has a lot of complexity that isn't necessary to properly configure Motion; this last statement is an understatement in my opinion since I believe we can reduce configure.ac to at least 25% of its current size if we utilize proper Autotools functionality and style.

Starting from scratch and reimplementing configure.ac from the bottom up based on specifications should be a good way to develop a new sane configure.ac -- the existing configure.ac is so tangled up with complexity that it'll prove too cumbersome to clean up directly.

Let's use this issue as a tracker for the functionality that a new configure.ac should provide:

[x] Motion core dependencies checking
- [x] Check for libmicrohttpd library
- [x] Check for libjpeg library
- [x] Check for pthreads
  - [x] Check for pthread_setname_np function
  - [x] Check for pthread_getname_np function
- [x] Check for strerror_r function
[x] Define sysconfdir symbol
[x] Handle installation of gettext PO files
[x] Handle all files for make dist
- [x] Install manpages
- [x] Install doc and data files

Motion feature flags:

[x] --without-bktr Exclude to use bktr subsystem for BSD
[x] --without-v4l2 Disable V4L2 devices
[x] --with-webp Compile with Webp image support
[x] --without-mmal Compile without RaspberyPi mmal camera support
[x] --with-ffmpeg Build with FFMPEG support
[x] --without-sqlite3 Disable sqlite3 support in motion.
[x] --without-mysql Disable mysql support
[x] --without-mariadb Disable mariadb support
[x] --without-pgsql Disable pgsql support

Non-essential flags (implementing these flags is low-priority):

[x] --without-optimizecpu Exclude autodetecting platform and cpu type. This will disable the compilation of gcc optimizing code by platform and cpu.
[x] --with-developer-flags Causes practically all of the possible gcc warning flags to be set

Relatively speaking, this is not a lot of configuration options to support. So if we take a divide and conquer approach, we should have a clear and clean new configure.ac that is much easier to maintain.

Testing-related tasks:

[x] Reimplement test_builds.sh as part of make check
[x] Verify VPATH builds (out-of-tree building functionality)

vilhelmgray commented 5 years ago

I've opened pull request #956 to consolidate the patches necessary for a new configure.ac setup. If anyone would like to contribute, please submit pull requests to my fork repository based on its new_configure branch: https://github.com/vilhelmgray/motion/tree/new_configure

tosiara commented 5 years ago

After you are done please verify that out of tree builds are working as per this comment: https://github.com/Motion-Project/motion/pull/453#issuecomment-320617208

Mr-DaveDev commented 5 years ago

For reference.

vilhelmgray commented 5 years ago

@Mr-DaveDev Thank you, it looks like your changes should resolve MariaDB detection problem in issue #875 so it'll be good to merge in those changes. If a new release is made with those changes, I should be able to close the Gentoo Linux bugzilla issue that started this discussion! :-)

However, in the long-term I disagree the design of your configure.ac file. I believe there's still complexity in it that can be shaved off (for example, in https://github.com/Motion-Project/motion/pull/956/commits/7df879a5a3a2e0129dbfcec149e4fd5414fc987f I added the checks for pthreads in just 6 lines by leveraging the AX_PTHREAD macro).

Furthermore, I think the architecture of the configure script can be improved by changing the design of the configure.ac file to perform checks according to the standard Autoconf Input Layout -- this will allow finer control of the checking mechanisms by sectioning off each check/configuration according to its build system function rather than its external package lineage.

Take a look at https://github.com/Motion-Project/motion/pull/956/commits/b51e5d498465c923f03c3202ec927b901bcd69d6 to see what I mean: we have 4 distinct sections (search for libraries, search for headers, reconcile search results, and print results to user) which allows us better control when systems overlap (e.g. a user will still know they need the vcos library even if the failure occurs for the mmal_core library check).

For these reasons, I will continue with the build system overhaul in the #956 pull request. When you merge in your improvements from the 20190705_autotools branch into master, I'll rebase my branch on top of it so that I can take in your improvements as well.

I apologize for the amount of time it's taking -- adding the required checks isn't difficult from a technical standpoint, but I'm only able to add in a few checks at a time during my free time between work assignments, so it's a slow gradual process. However, I'm trying to complete a new checkmark every few days or so, so I estimate I'll have the pull request ready for testing and merge sometime within this month.

vilhelmgray commented 5 years ago

@Mr-DaveDev There is one more improvement you may consider before merging your branch. It's unnecessary to call pkg-config manually within configure.ac since pkg-config comes with its own set of Autoconf macros such as PKG_CHECK_EXIST and PKG_CHECK_MODULES. They are documented in the pkg-config man page.

Mr-DaveDev commented 5 years ago

Reviewing the PR, I would be curious as to how to specify different directories than the standard locations. This is pretty important for packages such as ffmpeg which change frequently.

vilhelmgray commented 5 years ago

That's a fair concern, especially when multiple compatible libraries such FFmpeg and Libav are installed on the same system, users should be able to specify which package they want. Luckily, Autoconf is designed to be pretty robust in this respect, and is setup to use the system environment variables to define its search path.

Executing ./configure --help lists the following documentation about influential environment variables:

Some influential environment variables:
  CC          C compiler command
  CFLAGS      C compiler flags
  LDFLAGS     linker flags, e.g. -L<lib dir> if you have libraries in a
              nonstandard directory <lib dir>
  LIBS        libraries to pass to the linker, e.g. -l<library>
  CPPFLAGS    (Objective) C/C++ preprocessor flags, e.g. -I<include dir> if
              you have headers in a nonstandard directory <include dir>
  CPP         C preprocessor

As described in the help printout above, if you have libraries and headers in nonstandard directories, you may pass them done using the respective system environment variables. For example, suppose we have a custom installation of FFmpeg and Libav, installed respectively at /opt/ffmpeg and /opt/libav; the following lines may be used:

# Use custom FFmpeg installation
./configure CPPFLAGS=-I/opt/ffmpeg/include LDFLAGS=-L/opt/ffmpeg/lib
# Use custom Libav installation
./configure CPPFLAGS=-I/opt/libav/include LDFLAGS=-L/opt/libav/lib

If this custom installation directory is expected to be used regularly, the Autoconf manual suggests utilizing a config.site file to specify the default search directories. So you would create a new file /opt/config.site with the following inside:

# Set header files search directory
if test -z "$CPPFLAGS"; then
    CPPFLAGS=-I/opt/ffmpeg/include
    #CPPFLAGS=-I/opt/libav/include
    #CPPFLAGS=`pkg-config --cflags libavutil libavformat libavcodec libswscale libavdevice`
fi

# Set library search directory
if test -z "$LDFLAGS"; then
    LDFLAGS=-L/opt/ffmpeg/lib
    #LDFLAGS=-L/opt/libav/lib
    #LDFLAGS=`pkg-config --libs libavutil libavformat libavcodec libswscale libavdevice`
fi

Then you can execute the configure script simply as:

./configure CONFIG_SITE=/opt/config.site

This should only be necessary for users who installed their dependency packages to nonstandard locations -- most users will be using the package manager of their distro which should install packages to standard locations that Autoconf already knows.

However if you find it prudent, I can write a config.site with all the pkg-config calls written inside. We can distribute it in the Motion tarball so that users can just execute ./configure CONFIG_SITE=./config.site if they have a lot of custom installed packages and want to use pkg-config to configure the installation locations.

vilhelmgray commented 5 years ago

I consider pull request #956 to be feature-complete now.

I've tested the functionality as much as I could, but I do need someone to verify that the configure script works for Raspberry Pi and BSD since I lack both of those systems to test myself. In theory it should work since I have the configure script verify the presence of the respective headers and libraries.

In general, if you come across an error or otherwise strange behavior, please let me know and I'll update the pull request with a fix.

@tosiara Out-of-tree building can be performed using the canonical mkdir b && cd b && ./configure && make && make install.

@Mr-DaveDev Thank you for providing your 20190705_autotools, it really helped in understanding the changes necessary to the main code.

There is one set of changes that I did not understand, so I did not include in pull request #956: the ffmpeg.c changes; I'm not sure what's happening here with the switch from 4 to 5, so perhaps this is best added as separate patch.

Similarly, does the Motion source code actually require _GNU_SOURCE to be defined?

By the way, the Travis builds are failing right now due to missing Autotools-related dependencies for the build servers. I'll fix this in the coming days, but they are unrelated to the pull request code itself so you should be able to test this pull request on your machine without trouble.

Since pull request #956 reorganizes the directory structure of the Motion source code, it may trigger a merge conflict with some of the other currently pending pull requests you have. Do you prefer merging #956 first, or the other pull requests? If you want to merge #956 last, then let me know and I'll rebase to the latest master when you're ready.

If you have a question about the rationale for some of the changes, I'll be happy to answer.

For this pull request, I've tried to duplicate the functionality of the current motion configure script, while still updating it to the latest best practices of Autotools. There are further changes I would like to make, but since I do not want to make this pull request too complex, I'll delay them for after we merge in what we have now.

For reference, I'll list some of my intended future changes. After the merge, I'll open new issues for each of these so we can discuss and debate them more in-depth (I want to keep this issue focused instead on merging):

Utilize Autotools-generated INSTALL file. Autotools can automatically generate an INSTALL file with proper instructions on how to execute the configure script and makefiles. The existing Motion INSTALL file is essentially a simplified version of the Autotools-generated INSTALL file.
Do not check for optional features unless requested. Currently the configure script will check for optional features whether the user requested them or not. This behavior is problematic for two reasons.

First, the configure.ac file has added complexity in order to handle the checking behavior (e.g. LIBS values are carried to the end, additional cache variables must be utilized, etc.). The configure.ac file can be significantly simpler without these checks.

Second, this causes problems for package maintainers and cross-compilers. Often, a user may want to build the program on a different system than the one they run it. In this scenario, the build system may have features that the host system does not have -- but the Motion configure script will automatically enable those extra features, so the package maintainer is forced to provide --without- flags by default.

A more reasonable approach for the configure script is to keep it simple: only enable optional features explicitly requested. This simplifies our lives as developers (simple configure.ac file maintenance) , simplifies the lives of of package maintainers (simple package builds), and provides a standard behavior expected of an Autotools configure script (GNU packages follow a keep it simple approach). Realistically, a user is expected to read the output of ./configure --help to determine the flags they need for optional features.
Move --with-developer-flags and --with-optimizecpu out of the configure script. These flags set GCC compiler flags that are unrelated to the features of Motion. The purpose of the configure script is to detect and configure the functionality required to build the project. These two flags do not fit that purpose (they are unrelated to Motion features) and instead are purely settings for the compiler -- and worse, they are specifically GCC flags so they break the configuration for users of different compilers.

The functionality of these flags would be better implemented outside of the configure script (perhaps via their own scripts) and called when make is called to execute the compiler. This will allow users who want it to enable the developer flags if they have GCC, while preventing the script from causing problems for users who don't.
Improve tests scripts. Currently, the tests scripts work by reconfiguring and rebuilding Motion using various flags. However, these types of tests are not very useful since a successful build does not necessarily mean that the program will run as intended. The tests should be updated to test for functionality rather than successful building. See the Automake manual for more information about tests.
Replace -Dsysconfdir and -DLOCALEDIR with configuration file. Hardcoding these values using defines may not be a good idea since the location of these directories may change if the motion executable is moved to a new system, or if the current system is reorganizes to a new directory layout. Perhaps a Motion configuration file could be useful for getting these values.
Conditionally build source files. The MMAL and V4L2 related files are conditionally built; see src/Makefile.am. I suspect it's possible to do the same for some other files if we make a few changes in the source code. This may be worth investigating as it could significantly speed up the build time.

tosiara commented 5 years ago

autoreconf: Entering directory `.'
autoreconf: running: autopoint --force
Can't exec "autopoint": No such file or directory at /usr/share/autoconf/Autom4te/FileUtils.pm line 345.
autoreconf: failed to run autopoint: No such file or directory
autoreconf: autopoint is needed because this package uses Gettext

tosiara commented 5 years ago

New dependency?

vilhelmgray commented 5 years ago

autopoint is a dependency supplied by the Gettext package. See https://www.gnu.org/software/autoconf/manual/autoconf-2.68/html_node/autoreconf-Invocation.html

tosiara commented 5 years ago

So, this will be an additional dependency that should be mentioned in the documentation

tosiara commented 5 years ago

Ubuntu 14:

autopoint: *** The AM_GNU_GETTEXT_VERSION declaration in your configure.ac
               file requires the infrastructure from gettext-0.19 but this version
               is older. Please upgrade to gettext-0.19 or newer.
autopoint: *** Stop.
autoreconf: autopoint failed with exit status: 1

tosiara commented 5 years ago

Linux Mint 19.1:

$ ./configure 
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking for style of include used by make... GNU
checking dependency style of gcc... gcc3
checking for library containing jpeg_std_error... -ljpeg
checking for library containing MHD_start_daemon... -lmicrohttpd
./configure: line 4170: AX_PTHREAD: command not found
checking for library containing avcodec_find_decoder... no
checking for library containing avdevice_register_all... no
checking for library containing avformat_alloc_context... no
checking for library containing av_image_fill_arrays... no
checking for library containing sws_scale... no
checking for library containing mysql_init... no
checking for library containing mmal_component_create... no
checking for library containing mmal_port_parameter_set_boolean... no
checking for library containing vcos_assert... no
checking for library containing vc_gencmd... no
checking for library containing mysql_library_init... no
checking for library containing PQstatus... no
checking for library containing sqlite3_open... no
checking for library containing WebPEncode... no
checking for library containing WebPMuxAssemble... no
checking how to run the C preprocessor... /lib/cpp
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for ANSI C header files... no
checking for sys/types.h... no
checking for sys/stat.h... no
checking for stdlib.h... no
checking for string.h... no
checking for memory.h... no
checking for strings.h... no
checking for inttypes.h... no
checking for stdint.h... no
checking for unistd.h... no
checking jerror.h usability... no
checking jerror.h presence... yes
configure: WARNING: jerror.h: present but cannot be compiled
configure: WARNING: jerror.h:     check for missing prerequisite headers?
configure: WARNING: jerror.h: see the Autoconf documentation
configure: WARNING: jerror.h:     section "Present But Cannot Be Compiled"
configure: WARNING: jerror.h: proceeding with the compiler's result
checking for jerror.h... no
checking jpeglib.h usability... no
checking jpeglib.h presence... yes
configure: WARNING: jpeglib.h: present but cannot be compiled
configure: WARNING: jpeglib.h:     check for missing prerequisite headers?
configure: WARNING: jpeglib.h: see the Autoconf documentation
configure: WARNING: jpeglib.h:     section "Present But Cannot Be Compiled"
configure: WARNING: jpeglib.h: proceeding with the compiler's result
checking for jpeglib.h... no
checking microhttpd.h usability... no
checking microhttpd.h presence... yes
configure: WARNING: microhttpd.h: present but cannot be compiled
configure: WARNING: microhttpd.h:     check for missing prerequisite headers?
configure: WARNING: microhttpd.h: see the Autoconf documentation
configure: WARNING: microhttpd.h:     section "Present But Cannot Be Compiled"
configure: WARNING: microhttpd.h: proceeding with the compiler's result
checking for microhttpd.h... no
checking pthread_np.h usability... no
checking pthread_np.h presence... no
checking for pthread_np.h... no
checking dev/bktr/ioctl_meteor.h usability... no
checking dev/bktr/ioctl_meteor.h presence... no
checking for dev/bktr/ioctl_meteor.h... no
checking dev/ic/bt8xx.h usability... no
checking dev/ic/bt8xx.h presence... no
checking for dev/ic/bt8xx.h... no
checking dev/bktr/ioctl_bt848.h usability... no
checking dev/bktr/ioctl_bt848.h presence... no
checking for dev/bktr/ioctl_bt848.h... no
checking for dev/ic/bt8xx.h... (cached) no
checking linux/videodev2.h usability... no
checking linux/videodev2.h presence... yes
configure: WARNING: linux/videodev2.h: present but cannot be compiled
configure: WARNING: linux/videodev2.h:     check for missing prerequisite headers?
configure: WARNING: linux/videodev2.h: see the Autoconf documentation
configure: WARNING: linux/videodev2.h:     section "Present But Cannot Be Compiled"
configure: WARNING: linux/videodev2.h: proceeding with the compiler's result
checking for linux/videodev2.h... no
checking whether strerror_r is declared... no
checking for strerror_r... no
checking whether strerror_r returns char *... no
checking for pthread_getname_np... no
checking for pthread_setname_np... no
checking for a sed that does not truncate output... /bin/sed
checking whether NLS is requested... yes
checking for msgfmt... /usr/bin/msgfmt
checking for gmsgfmt... /usr/bin/msgfmt
checking for xgettext... /usr/bin/xgettext
checking for msgmerge... /usr/bin/msgmerge
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking for ld used by ... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for shared library run path origin... done
checking for CFPreferencesCopyAppValue... no
checking for CFLocaleCopyCurrent... no
checking for GNU gettext in libc... no
checking for iconv... no, consider installing GNU libiconv
checking for GNU gettext in libintl... no
checking whether to use NLS... no
configure: error: strerror_r function not found

vilhelmgray commented 5 years ago

~~It looks like you're using an old version of gettext, but I think Ubuntu 14 has gettext-0.19 in its repository so you may be able to do an apt update to get the latest version.~~ I'm sorry, I was looking at Ubuntu 16, I'm not sure if Ubuntu 14 has the gettext-0.19 package available in its repository.

@Mr-DaveDev I copied the requirement of gettext version 0.19 from your reference branch, but I'm not familiar enough with the Motion code to know whether 0.19 is actually the minimum needed. Would version 0.18 suffice for our needs?

vilhelmgray commented 5 years ago

@tosiara The Linux Mint system you're using is missing the pthread dev package which contains the AX_PTHREAD macro definition.

I'm going through the errors in the Travis builds to get a more exhaustive list of the Autotools dependencies necessary to run autoreconf to generate a proper configure script -- right now the errors you see are caused because it tosses unknown macro calls into the final script verbatim. Once I have the list, I'll post them here so we can update the documentation with a list of dependencies for Motion developers.

tosiara commented 5 years ago

Currently, on my Mint 19.1, I have all the dev packages required to compile motion 4.2.2 Which pthread dev package would I need else?

vilhelmgray commented 5 years ago

Try sudo apt-get install autoconf-archive on Linux Mint to get the AX_PTHREAD macro definition installed.

tosiara commented 5 years ago

Debian 10 "Buster":

$ autoreconf -fiv
autoreconf: Entering directory `.'
autoreconf: running: autopoint --force
autopoint: using AM_GNU_GETTEXT_REQUIRE_VERSION instead of AM_GNU_GETTEXT_VERSION
autoreconf: running: aclocal --force 
configure.ac:235: warning: macro 'AM_GNU_GETTEXT' not found in library
configure.ac:237: warning: macro 'AM_GNU_GETTEXT_VERSION' not found in library
configure.ac:238: warning: macro 'AM_GNU_GETTEXT_REQUIRE_VERSION' not found in library
autoreconf: configure.ac: tracing
autoreconf: configure.ac: AM_GNU_GETTEXT_VERSION is used, but not AM_GNU_GETTEXT
autoreconf: configure.ac: not using Libtool
autoreconf: running: /usr/bin/autoconf --force
configure.ac:245: error: possibly undefined macro: AM_GNU_GETTEXT
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
configure.ac:247: error: possibly undefined macro: AM_GNU_GETTEXT_VERSION
configure.ac:248: error: possibly undefined macro: AM_GNU_GETTEXT_REQUIRE_VERSION
autoreconf: /usr/bin/autoconf failed with exit status: 1

vilhelmgray commented 5 years ago

AM_GNU_GETTEXT is defined in gettext.m4 which should be installed when you install the Gettext package. Which version of the gettext package do you have on your Debian server?

tosiara commented 5 years ago

ii  autopoint                        0.19.8.1-9                   all          The autopoint program from GNU gettext
ii  gettext-base                     0.19.8.1-9                   amd64        GNU Internationalization utilities for the base system

vilhelmgray commented 5 years ago

Hmm, the Debian gettext package file list has it located at /usr/share/aclocal/gettext.m4. Do you have that file on your system, and does it have the definition of AM_GNU_GETTEXT inside?

tosiara commented 5 years ago

$ ls -l /usr/share/aclocal/gettext.m4
ls: cannot access '/usr/share/aclocal/gettext.m4': No such file or directory

tosiara commented 5 years ago

Had to install gettext manualy

vilhelmgray commented 5 years ago

Ah, I see the gettext-base and gettext packages are different in Debian.

tosiara commented 5 years ago

Configure fails:

$ ./configure 
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /usr/bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking whether make supports nested variables... yes
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking whether gcc understands -c and -o together... yes
checking whether make supports the include directive... yes (GNU style)
checking dependency style of gcc... gcc3
checking for library containing jpeg_std_error... -ljpeg
checking for library containing MHD_start_daemon... -lmicrohttpd
./configure: line 4137: AX_PTHREAD: command not found
checking for library containing avcodec_find_decoder... no
checking for library containing avdevice_register_all... no
checking for library containing avformat_alloc_context... no
checking for library containing av_image_fill_arrays... no
checking for library containing sws_scale... no
checking for library containing mysql_init... no
checking for library containing mmal_component_create... no
checking for library containing mmal_port_parameter_set_boolean... no
checking for library containing vcos_assert... no
checking for library containing vc_gencmd... no
checking for library containing mysql_library_init... no
checking for library containing PQstatus... no
checking for library containing sqlite3_open... no
checking for library containing WebPEncode... no
checking for library containing WebPMuxAssemble... no
checking how to run the C preprocessor... /lib/cpp
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for ANSI C header files... no
checking for sys/types.h... no
checking for sys/stat.h... no
checking for stdlib.h... no
checking for string.h... no
checking for memory.h... no
checking for strings.h... no
checking for inttypes.h... no
checking for stdint.h... no
checking for unistd.h... no
checking jerror.h usability... no
checking jerror.h presence... yes
configure: WARNING: jerror.h: present but cannot be compiled
configure: WARNING: jerror.h:     check for missing prerequisite headers?
configure: WARNING: jerror.h: see the Autoconf documentation
configure: WARNING: jerror.h:     section "Present But Cannot Be Compiled"
configure: WARNING: jerror.h: proceeding with the compiler's result
checking for jerror.h... no
checking jpeglib.h usability... no
checking jpeglib.h presence... yes
configure: WARNING: jpeglib.h: present but cannot be compiled
configure: WARNING: jpeglib.h:     check for missing prerequisite headers?
configure: WARNING: jpeglib.h: see the Autoconf documentation
configure: WARNING: jpeglib.h:     section "Present But Cannot Be Compiled"
configure: WARNING: jpeglib.h: proceeding with the compiler's result
checking for jpeglib.h... no
checking microhttpd.h usability... no
checking microhttpd.h presence... yes
configure: WARNING: microhttpd.h: present but cannot be compiled
configure: WARNING: microhttpd.h:     check for missing prerequisite headers?
configure: WARNING: microhttpd.h: see the Autoconf documentation
configure: WARNING: microhttpd.h:     section "Present But Cannot Be Compiled"
configure: WARNING: microhttpd.h: proceeding with the compiler's result
checking for microhttpd.h... no
checking pthread_np.h usability... no
checking pthread_np.h presence... no
checking for pthread_np.h... no
checking dev/bktr/ioctl_meteor.h usability... no
checking dev/bktr/ioctl_meteor.h presence... no
checking for dev/bktr/ioctl_meteor.h... no
checking dev/ic/bt8xx.h usability... no
checking dev/ic/bt8xx.h presence... no
checking for dev/ic/bt8xx.h... no
checking dev/bktr/ioctl_bt848.h usability... no
checking dev/bktr/ioctl_bt848.h presence... no
checking for dev/bktr/ioctl_bt848.h... no
checking for dev/ic/bt8xx.h... (cached) no
checking linux/videodev2.h usability... no
checking linux/videodev2.h presence... yes
configure: WARNING: linux/videodev2.h: present but cannot be compiled
configure: WARNING: linux/videodev2.h:     check for missing prerequisite headers?
configure: WARNING: linux/videodev2.h: see the Autoconf documentation
configure: WARNING: linux/videodev2.h:     section "Present But Cannot Be Compiled"
configure: WARNING: linux/videodev2.h: proceeding with the compiler's result
checking for linux/videodev2.h... no
checking whether strerror_r is declared... no
checking for strerror_r... no
checking whether strerror_r returns char *... no
checking for pthread_getname_np... no
checking for pthread_setname_np... no
checking for a sed that does not truncate output... /usr/bin/sed
checking whether NLS is requested... yes
checking for msgfmt... /usr/bin/msgfmt
checking for gmsgfmt... /usr/bin/msgfmt
checking for xgettext... /usr/bin/xgettext
checking for msgmerge... /usr/bin/msgmerge
checking build system type... x86_64-pc-linux-gnu
checking host system type... x86_64-pc-linux-gnu
checking for ld used by ... /bin/ld
checking if the linker (/bin/ld) is GNU ld... yes
checking for shared library run path origin... done
checking for CFPreferencesCopyAppValue... no
checking for CFLocaleCopyCurrent... no
checking for GNU gettext in libc... no
checking for iconv... no, consider installing GNU libiconv
checking for GNU gettext in libintl... no
checking whether to use NLS... no
configure: error: strerror_r function not found

tosiara commented 5 years ago

$ sudo apt-get install autoconf-archive did not help this time

vilhelmgray commented 5 years ago

The line ./configure: line 4137: AX_PTHREAD: command not found indicates that you are missing the ax_pthread.m4 file. On Debian, this should be provided by the autoconf-archive package.

Is there a /usr/share/aclocal/ax_pthread.m4 file on your system?

tosiara commented 5 years ago

Ah sorry, I had to re-run autoreconf -fiv after installing autoconf-archive. Now compiled on Buster

Mr-DaveDev commented 5 years ago

To address some of the questions/comments:

Utilize Autotools-generated INSTALL file. Autotools can automatically generate an INSTALL file with proper instructions on how to execute the configure script and makefiles. The existing Motion INSTALL file is essentially a simplified version of the Autotools-generated INSTALL file.

The key information that this provides is not the configure/ make information that the autotools would provide but instead the packages that are generally required for the most common distro.

Do not check for optional features unless requested. Currently the configure script will check for optional features whether the user requested them or not. This behavior is problematic for two reasons.

Most of the "features" are really driven from the functionality of the base distro. i.e. NetBSD, OpenBSD, FreeBSD, Alpine, BuildRoot, OpenWRT, whatever DBMS the user has etc. For example, most users would say that FFMpeg is not a feature but instead mandatory. Unfortunately, that is not all of the users. Some still just use a webcam and jpgs. Hence, FFMpeg is a optional feature. Also, simply eliminating the --with / --without and deciding to either include the option or not based upon the configure search results would be a real impediment.

First, the configure.ac file has added complexity in order to handle the checking behavior (e.g. LIBS values are carried to the end, additional cache variables must be utilized, etc.). The configure.ac file can be significantly simpler without these checks.

Second, this causes problems for package maintainers and cross-compilers. Often, a user may want to build the program on a different system than the one they run it. In this scenario, the build system may have features that the host system does not have -- but the Motion configure script will automatically enable those extra features, so the package maintainer is forced to provide --without- flags by default.

In the time on this project, I have not observed cross compiling to be "Often". It would seem to be more of an "exception" in what I've observed. This also seems to be an issue of whether the default shall be without versus with. In my opinion, the default should be to the most common option which I believe would be with

A more reasonable approach for the configure script is to keep it simple: only enable optional features explicitly requested. This simplifies our lives as developers (simple configure.ac file maintenance) , simplifies the lives of of package maintainers (simple package builds), and provides a standard behavior expected of an Autotools configure script (GNU packages follow a keep it simple approach). Realistically, a user is expected to read the output of ./configure --help to determine the flags they need for optional features.

I would argue that it would make the development of at least one developer significantly harder. This again falls into which is the most common option of those using the configure script. Is it --with or --without

Move --with-developer-flags and --with-optimizecpu out of the configure script. These flags set GCC compiler flags that are unrelated to the features of Motion. The purpose of the configure script is to detect and configure the functionality required to build the project. These two flags do not fit that purpose (they are unrelated to Motion features) and instead are purely settings for the compiler -- and worse, they are specifically GCC flags so they break the configuration for users of different compilers.

I would say that the configure script is supposed to configure the application. The --with-developer-flags is for building better application and removing bugs since it treats all warnings as errors and alerts the developers to potential errors. The requirement to identify XSI strerror_r is an example. The default however can not be to always have these on since it would cause additional difficulties for packagers.

The --with-optimizecpu is similar in that it is configuring the application and is particularly required when handling the Rasberry Pi. I believe Debian and Rasbian compile on a generic ARM platform which does not permit the optimization for NEON nor the identification of Pi2 vs Pi3, etc. While the intrinsic optimizations couldn't be pulled in for various reasons, the configure still needs to have some sort of option to turn on that feature of the code.

The functionality of these flags would be better implemented outside of the configure script (perhaps via their own scripts) and called when make is called to execute the compiler. This will allow users who want it to enable the developer flags if they have GCC, while preventing the script from causing problems for users who don't.

As the name of the options indicate, these are intended for a very limited subset of the users. I think it is also always better to have embedded/automated testing rather than external/manual.

Improve tests scripts. Currently, the tests scripts work by reconfiguring and rebuilding Motion using various flags. However, these types of tests are not very useful since a successful build does not necessarily mean that the program will run as intended. The tests should be updated to test for functionality rather than successful building. See the Automake manual for more information about tests.

I find these tests to be extremely useful especially when dealing with PRs as they highlight unforeseen development issues. i.e. You've coded for each of the various DBMS in all of the #IFs but did you account for the situation in which there no DBMS?. Did you consider the Apple builds or the distros without V4L2. With respect to program running as intended testing, this would seem problematic with a application requiring a camera.

Replace -Dsysconfdir and -DLOCALEDIR with configuration file. Hardcoding these values using defines may not be a good idea since the location of these directories may change if the motion executable is moved to a new system, or if the current system is reorganizes to a new directory layout. Perhaps a Motion configuration file could be useful for getting these values.

With the resources available to the project, there is no support for anyone who would move an executable. Any user who believes and expects it would be OK to just copy an executable across systems and have it work would need to be corrected in the error of their thinking. This would seem to apply to every application, not just Motion.

Conditionally build source files. The MMAL and V4L2 related files are conditionally built; see src/Makefile.am. I suspect it's possible to do the same for some other files if we make a few changes in the source code. This may be worth investigating as it could significantly speed up the build time.

In my observation of the project, the build time is not an impediment. The #IFs scattered in just about every single other module just to exclude a module is a very real impediment. I'm guessing I'm alone on this one but I find trying to follow the logic of code with 100's of #IFs breaking if statements, compounding among themselves and scattered to every calling function in multiple modules across the application as a bigger issue than the build time for the application.

@Mr-DaveDev I copied the requirement of gettext version 0.19 from your reference branch, but I'm not familiar enough with the Motion code to know whether 0.19 is actually the minimum needed. Would version 0.18 suffice for our needs?

I do not believe that there is anything in particular that requires 0.19 instead of 0.18. The version was just grabbed from the example I found.

There is one set of changes that I did not understand, so I did not include in pull request #956: the ffmpeg.c changes; I'm not sure what's happening here with the switch from 4 to 5, so perhaps this is best added as separate patch.

That is fine, I believe that was a carry over from a build warning.

Similarly, does the Motion source code actually require _GNU_SOURCE to be defined?

Most likely no. I believe it was added when it changed from configure.in to configure.ac and it eliminated some compiler warnings. Now that the code is is down to zero warnings, it may be possible to eliminate this and determine the root cause.

I've tested the functionality as much as I could, but I do need someone to verify that the configure script works for Raspberry Pi and BSD since I lack both of those systems to test myself. In theory it should work since I have the configure script verify the presence of the respective headers and libraries.

Configure logs from FreeBSD 11.2 are in this gist. There appears to be some searching/directory issues. I noticed CLANG as well as JPG and MySQL. Once the configure was specified with the directory for JPG found via the autotools branch version, it did configure and compile but didn't find MySQL. The autotools branch configure result is in this gist for reference. These were run one right after the other on a plain VM that is only used for testing. I also tried out Alpine and it was having some issues as well so getting the Travis updated should show those.

Since pull request #956 reorganizes the directory structure of the Motion source code, it may trigger a merge conflict with some of the other currently pending pull requests you have. Do you prefer merging #956 first, or the other pull requests? If you want to merge #956 last, then let me know and I'll rebase to the latest master when you're ready.

Going through the list, I am not sure which of the current PRs may conflict are even still viable. I do not believe that the UVC (#804) is thread safe and the videoio.h (#904) needs a lot more testing to ensure it does not break FreeBSD/NetBSD and also functions on OpenBSD.

I would anticipate that this would need to go in first since it would then cascade over to a version 4.2.3. I will then determine whether the resulting additional requirements this invokes is something that I want to continue spending my time upon.

vilhelmgray commented 5 years ago

Conditionally build source files. The MMAL and V4L2 related files are conditionally built; see src/Makefile.am. I suspect it's possible to do the same for some other files if we make a few changes in the source code. This may be worth investigating as it could significantly speed up the build time.

In my observation of the project, the build time is not an impediment. The #IFs scattered in just about every single other module just to exclude a module is a very real impediment. I'm guessing I'm alone on this one but I find trying to follow the logic of code with 100's of #IFs breaking if statements, compounding among themselves and scattered to every calling function in multiple modules across the application as a bigger issue than the build time for the application.

I agree with you, the many preprocessor conditionals make it difficult to follow the logic of the program. One of the advantages of using Automake to conditionally build files is that we can move all that code into dedicated files so that we no longer need the #IF lines everywhere. In this case they will all be self-contained in their respective files and compiled in only when the features are enabled.

However, it will take some time to remove all those #IF lines and reorganizing the code, so this is more of a long-term goal I think. Now that we have the ability to conditionally build files, we can progressively move towards that goal and simplify the code so that the logic in the main files are easier to read without all those breaking preprocessor #IF lines.

@Mr-DaveDev I copied the requirement of gettext version 0.19 from your reference branch, but I'm not familiar enough with the Motion code to know whether 0.19 is actually the minimum needed. Would version 0.18 suffice for our needs?

I do not believe that there is anything in particular that requires 0.19 instead of 0.18. The version was just grabbed from the example I found.

In that case let's remove AM_GNU_GETTEXT_VERSION and AM_GNU_GETTEXT_REQUIRE_VERSION since I don't think the code actually requires any special version of gettext. This should prevent the minimum version issue @tosiara came across in Ubuntu 14.

I've tested the functionality as much as I could, but I do need someone to verify that the configure script works for Raspberry Pi and BSD since I lack both of those systems to test myself. In theory it should work since I have the configure script verify the presence of the respective headers and libraries.

Configure logs from FreeBSD 11.2 are in this gist. There appears to be some searching/directory issues. I noticed CLANG as well as JPG and MySQL. Once the configure was specified with the directory for JPG found via the autotools branch version, it did configure and compile but didn't find MySQL. The autotools branch configure result is in this gist for reference. These were run one right after the other on a plain VM that is only used for testing. I also tried out Alpine and it was having some issues as well so getting the Travis updated should show those.

Thank you for the gists, I'll check out where it's failing and update the PR with a fix.

From the debugging with @tosiara, I believe the only new dependency is autoconf-archive. This is needed by developers in order to properly generate the configure script via autoreconf, but normal end-users who just execute ./configure from the dist tarball won't require it.

I need to familiarize myself with the syntax for the build file since I'm new to Travis, but I'll update it in the coming days so we can have some valid tests. I'm having trouble right now getting the build servers to execute the apt-get install lines for the Motion dependencies. If you happen to know where I'm going wrong in the build file (or have a good Travis resource page) please let me know.

Since pull request #956 reorganizes the directory structure of the Motion source code, it may trigger a merge conflict with some of the other currently pending pull requests you have. Do you prefer merging #956 first, or the other pull requests? If you want to merge #956 last, then let me know and I'll rebase to the latest master when you're ready.

Going through the list, I am not sure which of the current PRs may conflict are even still viable. I do not believe that the UVC (#804) is thread safe and the videoio.h (#904) needs a lot more testing to ensure it does not break FreeBSD/NetBSD and also functions on OpenBSD.

I would anticipate that this would need to go in first since it would then cascade over to a version 4.2.3. I will then determine whether the resulting additional requirements this invokes is something that I want to continue spending my time upon.

That's a fair point, some of the current PRs are old as well so they may need to be updated regardless. In that case we can merge this PR first and have it be part of version 4.2.3.

vilhelmgray commented 5 years ago

@Mr-DaveDev I copied the requirement of gettext version 0.19 from your reference branch, but I'm not familiar enough with the Motion code to know whether 0.19 is actually the minimum needed. Would version 0.18 suffice for our needs?

I do not believe that there is anything in particular that requires 0.19 instead of 0.18. The version was just grabbed from the example I found.

In that case let's remove AM_GNU_GETTEXT_VERSION and AM_GNU_GETTEXT_REQUIRE_VERSION since I don't think the code actually requires any special version of gettext. This should prevent the minimum version issue @tosiara came across in Ubuntu 14.

It turns out AM_GNU_GETTEXT_VERSION is a required macro when AM_GNU_GETTEXT is used, so I won't be able to remove it. Instead, I can bring down the gettext version to the one available in the Ubuntu 14 repository.

However, is supporting Ubuntu 14 something we want to do? Ubuntu 14 has reached End of Life so it won't be getting updates anymore.

By the way, I'm using AM_GNU_GETTEXT_REQUIRE_VERSION so that users can use more recent versions of gettext, rather than the exact version requested. Unfortunately, this means developers will need support for this macro, which was only introduced in gettext version 0.19.6. This means for developers Debian Jessie won't be able to support it by default since the Jessie repository only carries version 0.19.3 (but of course a developer could manually install gettext version >= 0.19.6).

I've tested the functionality as much as I could, but I do need someone to verify that the configure script works for Raspberry Pi and BSD since I lack both of those systems to test myself. In theory it should work since I have the configure script verify the presence of the respective headers and libraries.

Configure logs from FreeBSD 11.2 are in this gist. There appears to be some searching/directory issues. I noticed CLANG as well as JPG and MySQL. Once the configure was specified with the directory for JPG found via the autotools branch version, it did configure and compile but didn't find MySQL. The autotools branch configure result is in this gist for reference. These were run one right after the other on a plain VM that is only used for testing. I also tried out Alpine and it was having some issues as well so getting the Travis updated should show those.

Thank you for the gists, I'll check out where it's failing and update the PR with a fix.

I'm suspecting that the FreeBSD jpeglib and mysql libraries and headers were installed in nonstandard locations -- i.e. locations not in the search path of the default FreeBSD C preprocessor. You can verify the search path of your C preprocessor with the following line: cpp -v /dev/null -o /dev/null

I'm installing a virtual machine with FreeBSD to verify this, so I'll report back after some testing.

For now, would you let me know if the following resolves the issue for you: ./configure CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib? I think the FreeBSD ports system is setup to install to those locations by default, so if they aren't part of the C preprocessor search path, the jpeglib and mysql headers and libraries won't be found.

From the debugging with @tosiara, I believe the only new dependency is autoconf-archive. This is needed by developers in order to properly generate the configure script via autoreconf, but normal end-users who just execute ./configure from the dist tarball won't require it.

I need to familiarize myself with the syntax for the build file since I'm new to Travis, but I'll update it in the coming days so we can have some valid tests. I'm having trouble right now getting the build servers to execute the apt-get install lines for the Motion dependencies. If you happen to know where I'm going wrong in the build file (or have a good Travis resource page) please let me know.

I resolved the issue by using the docker exec $(docker ps -aq) /bin/bash -c to execute the commands.

However, I noticed the ./configure --developer-flags causes the compilation to fail because of line 262 in the logger.c file. The line here causes an unused return value warning (-Wunused-result) in my version of GCC, and subsequently fails because of we're using -Werror in the developer-flags.

Since the cause of these warnings is unrelated to the changes for this pull request, should we postpone the fixes for these warnings until after merge?

tosiara commented 5 years ago

For the build break in logger.c, I have once resolved this in my fork. Maybe it is worth backporting upstream:

https://github.com/tosiara/motion/commit/2774aeb17dace44ea271b5b97447a4d8baf61e1f

Mr-DaveDev commented 5 years ago

It turns out AM_GNU_GETTEXT_VERSION is a required macro when AM_GNU_GETTEXT is used, so I won't be able to remove it. Instead, I can bring down the gettext version to the one available in the Ubuntu 14 repository.

However, is supporting Ubuntu 14 something we want to do? Ubuntu 14 has reached End of Life so it won't be getting updates anymore.

By the way, I'm using AM_GNU_GETTEXT_REQUIRE_VERSION so that users can use more recent versions of gettext, rather than the exact version requested. Unfortunately, this means developers will need support for this macro, which was only introduced in gettext version 0.19.6. This means for developers Debian Jessie won't be able to support it by default since the Jessie repository only carries version 0.19.3 (but of course a developer could manually install gettext version >= 0.19.6).

This would seem to be a problem. While it could possibly be argued to remove 14.04, not having it be supported on Jessie is concern since its EOL is not until June 2020.

I'm suspecting that the FreeBSD jpeglib and mysql libraries and headers were installed in nonstandard locations -- i.e. locations not in the search path of the default FreeBSD C preprocessor. You can verify the search path of your C preprocessor with the following line: cpp -v /dev/null -o /dev/null

There wasn't anything customized on this VM and the pkg-config did find the appropriate flags.

For now, would you let me know if the following resolves the issue for you: ./configure CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib? I think the FreeBSD ports system is setup to install to those locations by default, so if they aren't part of the C preprocessor search path, the jpeglib and mysql headers and libraries won't be found.

It resolved it for jpeglib but not MySQL. Note the additional sub-directory specified in the gist when using pkg-config. I personally do not feel this is a step forward by having to specify the directories this way. Especially since it also applies to use of different ffmpeg versions which is a extremely frequent need during development.

However, I noticed the ./configure --developer-flags causes the compilation to fail because of line 262 in the logger.c file. The line here causes an unused return value warning (-Wunused-result) in my version of GCC, and subsequently fails because of we're using -Werror in the developer-flags.

This is a error hiding as a warning and is an example of why to treat warnings as errors. Reading the note above the line of code referenced indicates the root cause and this was resolved in the previous configure script by testing for XSI_STRERROR_R

Since the cause of these warnings is unrelated to the changes for this pull request, should we postpone the fixes for these warnings until after merge?

I have thought about this further and determined that I will not be merging this. I will let the other motion-project members evaluate it. They can merge it in if they feel appropriate.

vilhelmgray commented 5 years ago

By the way, I'm using AM_GNU_GETTEXT_REQUIRE_VERSION so that users can use more recent versions of gettext, rather than the exact version requested. Unfortunately, this means developers will need support for this macro, which was only introduced in gettext version 0.19.6. This means for developers Debian Jessie won't be able to support it by default since the Jessie repository only carries version 0.19.3 (but of course a developer could manually install gettext version >= 0.19.6).

This would seem to be a problem. While it could possibly be argued to remove 14.04, not having it be supported on Jessie is concern since its EOL is not until June 2020.

This won't affect end-users since the configure script itself just depends on portable POSIX utilities common available on all systems. This is only a problem for a Motion developer who happens to run Jessie, and then it's simple to install a more recent version of gettext via wget && tar && ./configure && make && make install (I can update the Travis build to include this extra step).

Alternatively, what other projects that use AM_GNU_GETTEXT_REQUIRE_VERSION have done is include the definition file gettext.m4 in their repository so that developers won't need to download and install it manually. However, since Debian Jessie is the only system we've encountered thus far lacking it, and it's EOL next year, it may not be worth the effort to add gettext.m4 into the Motion repository just to remove it next year.

I'm suspecting that the FreeBSD jpeglib and mysql libraries and headers were installed in nonstandard locations -- i.e. locations not in the search path of the default FreeBSD C preprocessor. You can verify the search path of your C preprocessor with the following line: cpp -v /dev/null -o /dev/null

There wasn't anything customized on this VM and the pkg-config did find the appropriate flags.

It looks like FreeBSD has chosen to default pkg/ports installation path to be /usr/local/ since the FreeBSD developers do not consider them to be part of the operating system. The FreeBSD world /usr/local directory is akin to the Linux world /opt directory. I think this is because the FreeBSD philosophy expects the user to know what packages they have installed, since they selected those packages via pkg install or otherwise.

For now, would you let me know if the following resolves the issue for you: ./configure CPPFLAGS=-I/usr/local/include LDFLAGS=-L/usr/local/lib? I think the FreeBSD ports system is setup to install to those locations by default, so if they aren't part of the C preprocessor search path, the jpeglib and mysql headers and libraries won't be found.

It resolved it for jpeglib but not MySQL. Note the additional sub-directory specified in the gist when using pkg-config. I personally do not feel this is a step forward by having to specify the directories this way. Especially since it also applies to use of different ffmpeg versions which is a extremely frequent need during development.

FFmpeg shouldn't be an issue if the installation directories are provided as well. This is the same reason MySQL wasn't found: it appears to be located under /usr/local/lib/mysql and not /usr/local/lib.

If you want to use pkg-config in order to provide the installation directories then the configure line for just jpeglib and mysql would be:

./configure CPPFLAGS=`pkg-config --cflags libjpeg mysqlclient` LDFLAGS=`pkg-config --libs libjpeg mysqlclient`

I'm not as familiar with FreeBSD protocol as I am with Linux, so this will warrant some investigation to discover what the best practice is for projects using Autotools -- it's commonly used enough that there should be some example of how other projects are handling this situation. Whether FreeBSD users are expected to add their package installation locations to their global config.site configuration file, or whether the project being installed is expected to provide its own config.site configuration.

One thing that is clear however is that Autotools does not recommend embedding pkg-config use into the configure script itself, but that it should be used externally. I was wrong in my earlier suggestion of PKG_CHECK_MODULES, and William Pursell goes into detail about the issues that arise from this method.

One of the problems for example is that the configure script will have to verify that the library is not already installed in the standard search path via AC_CHECK_LIBS, then call PKG_CHECK_MODULES for pkg-config, and finally verify that the pkg-config printout is actually correct (since the .pc file can realistically be out-of-sync with the package installation) by calling AC_CHECK_LIBS a second time. This creates a lot of redundancy and complexity in the configure script.

A more detailed response from the Autoconf developers about pkg-config extensions is available in their mailing list. Essentially the idea is to pass in the nonstandard installation directories, whether through a config.site file, wrapper script, or manually by the user.

If many Motion users on FreeBSD use pkg-config, then we can provide our own local Motion config.site file for convenience that has the necessary pkg-config commands for the packages in Motion. This will allow the Motion configure script to remain portable without a dependency on pkg-config, as well as prevent errors for users whose pkg-config is misconfigured and prints out incorrect installation locations.

However, I noticed the ./configure --developer-flags causes the compilation to fail because of line 262 in the logger.c file. The line here causes an unused return value warning (-Wunused-result) in my version of GCC, and subsequently fails because of we're using -Werror in the developer-flags.

This is a error hiding as a warning and is an example of why to treat warnings as errors. Reading the note above the line of code referenced indicates the root cause and this was resolved in the previous configure script by testing for XSI_STRERROR_R

The code for that particular line hasn't been changed (though I did flip the logic of the #if), so this warning should be present in the current Motion code as well if the line is compiled in. The fix here is to check the return value of strerror_r for an error state, as in the patch provided by @tosiara.

vilhelmgray commented 5 years ago

The latest version now passes all the Travis jobs. Actually, the Debian Jessie job is reported as a failure, but that is due to some sort of permissions issue (it fails to write a log file) -- however, from the Travis job log you can see it's passed all Motion tests; I will investigate this further to see why it's having trouble writing the log file.

For the sake of testing, I've added the patch from @tosiara in commit 9adc262ac7dcfddb603f404e48c618dd23338142 to this pull request in order to get past the build errors for the Travis tests.

I also discovered that pthread_setname_np, pthread_getname_np, and memmem require _GNU_SOURCE to be defined, so I've added AC_USE_SYSTEM_EXTENSIONS to resolve that issue.

Regarding the FreeBSD installation issue, I noticed the following line:

checking how to link with libiconv... /usr/local/lib/libiconv.so -Wl,-rpath -Wl,/usr/local/lib

So AM_GNU_GETTEXT was able to find libiconv despite it being installed in /usr/local/lib. I investigated further to figure out how it was doing this and discovered these configuration flags were acquired from AM_ICONV_LINKFLAGS_BODY in iconv.m4, which has the following code:

dnl Search for libiconv and define LIBICONV, LTLIBICONV and INCICONV
dnl accordingly.
AC_LIB_LINKFLAGS_BODY([iconv])

AC_LIB_LINKFLAGS_BODY is defined inside lib-link.m4, and this macro does check for libraries inside a few popular installation locations such as /usr/local/lib, so perhaps we can use it to resolve the issue with FreeBSD.

vilhelmgray commented 5 years ago

@Mr-DaveDev I've made updates to address the concerns you listed. Please check out the latest version of pull request #956.

In particular, FreeBSD packages will be detected by default now thanks to the use of AC_LIB_HAVE_LINKFLAGS. In addition, this creates a configure script option to allow users to specify a custom prefix for the installation location of the library (--with-<LIBRARY>-prefix=[DIR]).

For example, if you made a custom installation of PostgreSQL to /opt/pgsql, you can execute the following to specify it to the configure script (it'll automatically determine the proper include and lib directories):

./configure --with-libpq-prefix=/opt/pgsql

These are based off the name of the library file so to specify a custom FFmpeg installation path you would do the following:

./configure --with-libavdevice-prefix=/opt/ffmpeg

I've also made several miscellaneous updates, improvements, and bugfixes throughout the pull request, but nothing particularly interesting from a user perspective.

The Travis file has been updated and all jobs now pass. Debian Jessie comes with Automake version 1.14.1 in its repository which unfortunately has a bug in it regarding make check, so I'm having Travis install the latest Automake version 1.16.1 to resolve this.

I've rebased the pull request as well so that it's easily mergeable with the current master branch.

If you have any further objections or issues, please let me know and I'll see what I can do to adjust the code.

vilhelmgray commented 5 years ago

I've added a fix in commit https://github.com/Motion-Project/motion/pull/956/commits/79ef57405c49ca1226fc00af1995bdab287ae9ea of the pull request to address issue https://github.com/Motion-Project/motion/issues/995.

In short, MySQL has diverged from MariaDB in its option configuration protocol: MySQL no longer supports the my_bool datatype (instead favoring C99 bool), while MariaDB has deprecated the mysql_options function in favor of the mysql_optionsv function.

tosiara commented 4 years ago

@vilhelmgray , do you have any ideas about this message: https://github.com/Motion-Project/motion/issues/1018

vilhelmgray commented 4 years ago

Is there any interest still in pull request #956? If so, I will rebase on top of the latest Motion master branch; otherwise I'll abandon this solution and close the pull request. Just let me know what you prefer.