Skip to content

Fix emscripten build#258

Open
liulongbiao wants to merge 5 commits intoaubio:masterfrom
liulongbiao:fix-emscripten-build
Open

Fix emscripten build#258
liulongbiao wants to merge 5 commits intoaubio:masterfrom
liulongbiao:fix-emscripten-build

Conversation

@liulongbiao
Copy link

@liulongbiao liulongbiao commented Aug 28, 2019

Fixed some issue when I use docker to build aubio with emscripten.

Now we can use Docker to build WebAssembly module with :

docker run --rm -v $(pwd):/src trzeci/emscripten /bin/bash -c "./scripts/get_waf.sh && ./scripts/build_emscripten"

@liulongbiao
[build] update emcc check in cpp_cmd
ed8a2dd 
CC may be resolved as absolute path, such as /emsdn_portable/sdk/emcc.
And add empty __init__.py files.
@liulongbiao
[build] encode utf8 for c_namgled_names
bf1ad06 
the default encoding is utf8, and will print like `u'xxx'`
@liulongbiao
add some LINKFLAGS for product
467d9b4
@liulongbiao
[build] skip tests and docs for emscripten
1c71ba7 
Some tests needs dynamic library.
Since default build will test and generate docs, so emscripten build just need the wasm module
@liulongbiao
[build] add emscripten EXPORT_NAME
9092747 
use 'aubio' as exported module name
@piem
Copy link
Member

piem commented Aug 28, 2019

hi @liulongbiao,

thanks! looks good to me, except the first commit. will comment in details.

cheers, piem

piem
piem reviewed Aug 28, 2019
View reviewed changes
Copy link
Member

@piem piem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. why would you need to create the __init__.py files? these directories are not python modules
  2. what's missing with the current test?
>>> 'emcc' in '/path/to/emcc -E'
True

piem
piem reviewed Aug 28, 2019
View reviewed changes
Copy link
Member

@piem piem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tests and examples were correctly build last time i checked, and could be run with node. why disable them? see wscript#L528

piem
piem reviewed Aug 28, 2019
View reviewed changes
Copy link
Member

@piem piem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you describe a configuration where the list c_mangled_names is not generated correctly?

@liulongbiao
Copy link
Author

liulongbiao commented Aug 29, 2019

  1. why would you need to create the __init__.py files? these directories are not python modules
  2. what's missing with the current test?
>>> 'emcc' in '/path/to/emcc -E'
True

without the __init__.py files, when configure with --with-target-platform emscripten, will got:

  File "/src/wscript", line 275, in configure
    from python.lib.gen_external import get_c_declarations, \
ImportError: No module named python.lib.gen_external

@liulongbiao
Copy link
Author

liulongbiao commented Aug 29, 2019

tests and examples were correctly build last time i checked, and could be run with node. why disable them? see wscript#L528

When I execute the script scripts/build_emscripten with tests, I got :

[ 85/223] Compiling tests/src/spectral/test-fft.c
[ 86/223] Compiling tests/src/spectral/test-filterbank.c
emcc:WARNING: ignoring unsupported linker flag: `--enable-auto-import`
emcc:WARNING: ignoring unsupported linker flag: `-h`
emcc:WARNING: ignoring unsupported linker flag: `aubio.min.js.5`
emcc:WARNING: ignoring unsupported linker flag: `-Bstatic`
emcc:WARNING: ignoring unsupported linker flag: `-Bdynamic`

[ 87/223] Compiling tests/src/spectral/test-filterbank_mel.c
[ 88/223] Compiling tests/src/spectral/test-mfcc.c
[ 89/223] Linking build/tests/test-sink.js
shared:ERROR: emcc: cannot find library "aubio" (`-s ERROR_ON_MISSING_LIBRARIES=0` to disable this error)

Waf: Leaving directory `/src/build'
Build failed
 -> task in 'test-sink' failed with exit status 1 (run with -v to display more information)

@piem
Copy link
Member

piem commented Sep 24, 2019

hi @liulongbiao

please don't close this issue until it is resolved. i will have a deeper look. thanks for your patience,

Paul

@piem piem reopened this Sep 24, 2019
@codecov
Copy link

codecov bot commented Sep 24, 2019 •
edited
Loading

Codecov Report

Merging #258 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #258   +/-   ##
=======================================
  Coverage   88.09%   88.09%           
=======================================
  Files         124      124           
  Lines        9716     9716           
  Branches     1729     1729           
=======================================
  Hits         8559     8559           
  Misses       1110     1110           
  Partials       47       47

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 63fc1df...9092747. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@piem piem piem left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@liulongbiao @piem