bugGnash - The GNU Flash player - Bugs: bug #50174, `make check` failed, due to...

 
 

bug #50174: `make check` failed, due to ming-related link error

Submitter:  Nutchanon Wetchasit <nachanon>
Submitted:  Sun 29 Jan 2017 10:02:32 AM UTC
   
 
Category:  testsuite Severity:  3 - Normal
Release:  master Status:  Fixed
Privacy:  Public Assigned to:  strk
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 30 Jan 2017 04:41:28 PM UTC, comment #9: 

Thanks, pushed as 49ad8249663616938ad34edf35825d93a31740ff

Sandro Santilli <strk>
Group Member
Mon 30 Jan 2017 03:42:02 PM UTC, comment #8: 

From the inconsistencies mentioned in the previous post, I have attached a cosmetic patch `0002_improve-ming-configure-version-consistency.patch`, which rectify these inconsistencies in the configure script.

Note that this patch is not essential, and could be left out without any ill effect.

Gnash: 0.8.11dev (patched against git 37044f9 30-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386


(file #39612)

Nutchanon Wetchasit <nachanon>
Mon 30 Jan 2017 03:39:14 PM UTC, comment #7: 

Current git version of Gnash testsuite is now compiling successfully and showing normaml result, thanks.

Configure, build, and test logs are attached for the record, as `gnashbuildlog.37044f9.zip`. (Note that 2 test failures are expected in my configuration, per bug #47906)

Regarding the sed pattern, I actually went with that simple one on my the first try (s/^0*//), but decided to change it to the submitted one (s/^0*\(.*.\)$/\1/g) after I realized that the simple one tried to strip "00000000" into a blank "" instead of single zero "0".

The simplified pattern:

$ echo "12345678" | sed --posix 's/^0*//'
12345678
$ echo "00000010" | sed --posix 's/^0*//'
10
$ echo "00040400" | sed --posix 's/^0*//'
40400
$ echo "00000000" | sed --posix 's/^0*//'

$


Pattern from the submitted patch:

$ echo "12345678" | sed --posix 's/^0*\(.*.\)$/\1/g'
12345678
$ echo "00000010" | sed --posix 's/^0*\(.*.\)$/\1/g'
10
$ echo "00040400" | sed --posix 's/^0*\(.*.\)$/\1/g'
40400
$ echo "00000000" | sed --posix 's/^0*\(.*.\)$/\1/g'
0
$


You would see that the simple pattern does not handle "00000000" correctly, while the submitted pattern does.

But after testing, this problem turned out not to matter as much as I originally thought; this is because the configure script actually contain a check for empty MING_VERSION_CODE and automatically re-assign it with `00000000`. (Notice that the reassignment and comparisons around there are still using leading zeroes)

Gnash: 0.8.11dev (git 37044f9 30-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386


(file #39611)

Nutchanon Wetchasit <nachanon>
Mon 30 Jan 2017 08:14:09 AM UTC, comment #6: 

Your patch was committed as 04cbec56c6070bef7bafd0763a05921dca1e1d12

My simplification of sed as
37044f9ef5888401b2cfc83df8ae0ceeef6bd37c

Sandro Santilli <strk>
Group Member
Mon 30 Jan 2017 07:51:53 AM UTC, comment #5: 

Great work, thanks !
I think the sed line could be simplified to 's/^0*//'. I can do that in a separate commit unless you see any reason why it should not work...

Sandro Santilli <strk>
Group Member
Sun 29 Jan 2017 01:59:47 PM UTC, comment #4: 

From all said in previous posts, following changes need be introduced (on top of culprit commit) to prevent testsuite build error and other version detection anomalies:

  • `macros/ming.m4` needs to be changed to trim leading zeroes from `MING_VERSION_CODE` (possibly by passing it through `sed`).
  • Leading zeros need to be trimmed from `#if MING_VERSION_CODE` comparison in following files:
    • testsuite/actionscript.all/enumerate.as
    • testsuite/actionscript.all/Matrix.as
    • testsuite/actionscript.all/delete.as
    • testsuite/actionscript.all/Try.as
    • testsuite/actionscript.all/targetPath.as
    • testsuite/actionscript.all/check.as
    • testsuite/misc-ming.all/matrix_test.c
    • testsuite/misc-ming.all/attachMovieTest.c
    • testsuite/misc-ming.all/duplicate_movie_clip_test.c


(One can use `grep -nR MING_VERSION_CODE * | grep -Pi '\.(as|h|cc|cpp|c):'` in Gnash source directory to find all of these, save for the M4 macro)

See the attached `0001_fix-ming-octal-version-inconsistency.patch` for a patch which do these changes. On my machine, this patch allows `make check` to complete with normal results; but not sure if my use of `sed` is portable though.

Alternatively, the ff92e0a commit could be reverted (which works, in spite of the octal number limitation).

Gnash: 0.8.11dev (git ff92e0a 28-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386


(file #39605)

Nutchanon Wetchasit <nachanon>
Sun 29 Jan 2017 11:32:53 AM UTC, comment #3: 

Before commit ff92e0a, things are working fine as long as the value of each digit in Ming's major/minor/micro/prerelease version number does not exceed 7: the C preprocessor can just pretend they are all octal values and still do correct comparisons.

However, when ff92e0a is introduced...

  • `./configure` and `Makefile`(s) still do `#define MING_VERSION_CODE 00040400` (equivalent to 16640 in decimal)...
  • But some code are now changed to do comparision in decimal:
    • ming_utils.h:26: `#if 00040400 >= 40004` now became false (it should have been true).
    • ming_utils.h:43: Change has no effect (it's code comment).
    • ming_utils.h:45: `#if 00040400 >= 40004` now became false (it should have been true).
    • ming_utils.h:56: `#if 00040400 >= 40006` now became false (it should have been true).


From the link error:

registerClassTest2.c: In function 'main':
registerClassTest2.c:177:3: warning: implicit declaration of function 'add_clip_init_actions' [-Wimplicit-function-declaration]
registerClassTest2.c:192:5: warning: 'compileSWFActionCode' is deprecated (declared at /usr/include/ming.h:619) [-Wdeprecated-declarations]
registerClassTest2.c:199:5: warning: 'compileSWFActionCode' is deprecated (declared at /usr/include/ming.h:619) [-Wdeprecated-declarations]
  CCLD   registerClassTest2
registerClassTest2.o: In function `main':
/home/window/prog/gnash.git/testsuite/misc-ming.all/register_class/registerClassTest2.c:177: undefined reference to `add_clip_init_actions'
collect2: error: ld returned 1 exit status
make[5]: *** [registerClassTest2] Error 1


A bit deeper inspection reveals that this is a direct result of version interpretation discrepancy between C-preprocessor and configure script:

If libming version is actually less than 0.4.4, `testsuite/misc-ming.all/register_class/Makefile.am` would just exclude the build of `registerClassTest2` test in the configure process, as these libming did not support init action insertion.

But what really happened here is:
** `./configure` correctly identified that libming supports init action insertion (`test` always do comparison in base 10), resulting in `testsuite/misc-ming.all/register_class/registerClassTest2.c` being listed for building in Makefile.
* But `testsuite/misc-ming.all/ming_utils.h` (which is included by C source code above), *misidentified that libming does not support init action insertion, causing `add_clip_init_actions()` implementation to be excluded from `testsuite/misc-ming.all/ming_utils.c` and associated header.
** As `testsuite/misc-ming.all/register_class/registerClassTest2.c` tried to use `add_clip_init_actions()`, it results in implicit declaration warning.
* When linker linked `registerClassTest2.o` against `libgnashmingutils.a`, *it failed as `add_clip_init_actions()` function is missing, causing `make check` to abort.

Gnash: 0.8.11dev (git ff92e0a 28-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Sun 29 Jan 2017 10:58:35 AM UTC, comment #2: 

From observation under my configuration, `MING_VERSION_CODE` is not defined by libming header, but is defined by `Makefile`(s) as a result of `./configure` process; which is done as follows:

  if test x"$MING_CONFIG" != "x"; then
    MING_VERSION=`$MING_CONFIG --version`
    major=`echo $MING_VERSION | cut -d '.' -f 1`
    minor=`echo $MING_VERSION | cut -d '.' -f 2`
    micro=`echo $MING_VERSION | cut -d '.' -f 3`
    beta=`echo $MING_VERSION | sed -ne 's/.*beta\([[0-9]]*\).*/\1/p'`
    dnl This is a little screwy. The previous Ming release was tagged beta5,
    dnl but the newer one is tagged rc1. This makes it look like rc1 is older the
    dnl beta 5 release, as it only looks at the last integer. So we fudge the
    dnl numbers so tc1 comes after beta 5. ie.. this looks like beta 6.
    if test -z $beta; then
      beta=`echo $MING_VERSION | sed -ne 's/.*rc\([[0-9]]*\).*/\1/p'`
      if test x"$beta" != x; then
        beta=`eval expr $beta + 5`
      fi
    fi
    MING_VERSION_CODE=`printf %2.2d%2.2d%2.2d%2.2d $major $minor $micro $beta`
    MING_CFLAGS=`$MING_CONFIG --cflags`
    MING_LIBS=`$MING_CONFIG --libs`
    MING_PATH=`$MING_CONFIG --bindir`
    AC_PATH_PROG([MAKESWF], [makeswf], , [$MING_PATH:$PATH])
  fi


In my configuration, `MING_VERSION_CODE` became `00040400` in Makefile(s); which they, in turn, do something like this:

$(CXX) -some -options -DMING_VERSION_CODE=00040400 sourcefile.cpp


Which is equivalent to doing this in C-preprocessor:

#define MING_VERSION_CODE 00040400


Then, they are used for things like this in CPP-processed ActionScript and C code:

#if MING_VERSION_CODE >= 00040004


Gnash: 0.8.11dev (git ff92e0a 28-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Sun 29 Jan 2017 10:06:12 AM UTC, comment #1: 

Since the previous commit is compiled and tested fine. A use of git bisect (36028f4 good -> ff92e0a bad) is redundant, but I still use it anyway...

ff92e0ae816440b81ecb46215717a376a77491be is the first bad commit
commit ff92e0ae816440b81ecb46215717a376a77491be
Author: Sandro Santilli <strk@kbt.io>
Date:   Sat Jan 28 15:18:09 2017 +0100

    Fix invalid octal number in expansion of macro  MING_VERSION_CODE

    GCC 5.4.0 was complaining about it

:040000 040000 cc9e300978fe9c13b70f3c20b9efbeeac1519746 77e09588faca342e28f7d05e7b8fd1c8fa335cc2 M        testsuite


GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Sun 29 Jan 2017 10:02:32 AM UTC, original submission:  

Yesterday, while I was verifying in-tree correctness of patch #9234; I encountered a build error (link failure) during a `make check` run, which causes the whole testing process to abort. Note that only Gnash testsuite is affected, the Gnash build itself is still installable and functional.

The testsuite build error message is as follows:

.
.
.
  CCLD   registerClassTest2
registerClassTest2.o: In function `main':
/home/window/prog/gnash.git/testsuite/misc-ming.all/register_class/registerClassTest2.c:177: undefined reference to `add_clip_init_actions'
collect2: error: ld returned 1 exit status
make[5]: *** [registerClassTest2] Error 1
make[5]: Leaving directory `/home/window/prog/gnash.git/testsuite/misc-ming.all/register_class'
make[4]: *** [check-am] Error 2
make[4]: Leaving directory `/home/window/prog/gnash.git/testsuite/misc-ming.all/register_class'
make[3]: *** [check-recursive] Error 1
make[3]: Leaving directory `/home/window/prog/gnash.git/testsuite/misc-ming.all'
make[2]: *** [check-recursive] Error 1
make[2]: Leaving directory `/home/window/prog/gnash.git/testsuite'
make[1]: *** [check-recursive] Error 1
make[1]: Leaving directory `/home/window/prog/gnash.git'
make: *** [check] Error 2


My build procedure is as follows:

  • `git clean -xdf`
  • `git checkout -f`
  • `./autogen.sh`
  • `CC="ccache gcc" CXX="ccache g++" ./configure --enable-media=ffmpeg,gst --enable-renderer=agg,cairo,opengl --enable-gui=gtk,qt4,sdl,fb,dump --enable-docbook`
  • `make`
  • `make check`


Full configuration logs, build, and test output are attached as `mingcheckerror.ff92e0a.zip`.

Gnash: 0.8.11dev (git ff92e0a 28-Jan-2017)
GCC: 4.7.2-5 (debian)
G++: 4.7.2-5 (debian)
Ming: 0.4.4-1.1 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #39612:  0002_improve-ming-configure-version-consistency.patch added by nachanon (3KiB - text/x-diff - Patch to improve MING_VERSION_CODE consistency in configure script)
file #39611:  gnashbuildlog.37044f9.zip added by nachanon (61KiB - application/x-zip-compressed - Build and test log from Gnash 0.8.11dev git 37044f9)
file #39605:  0001_fix-ming-octal-version-inconsistency.patch added by nachanon (12KiB - text/x-diff - Patch to convert all MING_VERSION_CODE comparisons to decimal)
file #39601:  mingcheckerror.ff92e0a.zip added by nachanon (41KiB - application/x-zip-compressed - Full configuration logs, build, and test output)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by nachanon (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 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-01-30 strk StatusReady For Test Fixed
        Open/ClosedOpen Closed
    2017-01-30 nachanon Attached File- Added 0002_improve-ming-configure-version-consistency.patch, #39612
    2017-01-30 nachanon Attached File- Added gnashbuildlog.37044f9.zip, #39611
    2017-01-30 strk StatusNone Ready For Test
        Assigned toNone strk
    2017-01-29 nachanon Attached File- Added 0001_fix-ming-octal-version-inconsistency.patch, #39605
    2017-01-29 nachanon Attached File- Added mingcheckerror.ff92e0a.zip, #39601

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code