Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Feature #847674

Emscripten beach head.

Added by Sveinung Kvilhaugsvik over 2 years ago. Updated over 2 years ago.

Status:
Closed
Priority:
Normal
Category:
Bootstrap
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Split from Feature #838867. This patch is only enough to compile common. No complete emscripten binaries are produced for anything. I haven't checked if Feature #839110 can depend on this rather than on Feature #838867.

3.1.patch (9.54 KB) 3.1.patch Sveinung Kvilhaugsvik, 2019-11-17 12:11 AM
3.0.patch (9.53 KB) 3.0.patch Sveinung Kvilhaugsvik, 2019-11-17 01:40 AM

Related issues

Related to Freeciv - Feature #869747: emscripten.m4Closed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #869884: emscripten: Disable zlib configure checkClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #870131: emscripten: Disable fcdbClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #870324: emscripten: Disable libcurlClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #870642: emscripten: Disable icu configure checkClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocks Freeciv - Feature #838867: Add SDL2 Emscripten client to autotools buildNew

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocks Freeciv - Feature #847678: Export parts of common to Freeciv-webNew

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Sveinung Kvilhaugsvik over 2 years ago

  • Blocks Feature #838867: Add SDL2 Emscripten client to autotools build added

#2 Updated by Sveinung Kvilhaugsvik over 2 years ago

I have split out the 3.1 version. It is much easier to review two patches that each are size x/2 than one patch size x, given that one patch is one logical change. Please read it yourself to see if you find more issues now that it is shorter.

Issues found:
  • "IPv6 is not supported in Emscripten" printed when the server is requested to be built
  • FC_LD_FLAGS([""], [], [LDFLAGS]) - overwrites for all kinds of builds
  • not sure if this is an issue with the patch, with emscripten or expected and normal: a.out, a.out.js and a.out.wasm appears at the top in the build dir. Do you know why this happens?

#3 Updated by Sveinung Kvilhaugsvik over 2 years ago

#4 Updated by Sveinung Kvilhaugsvik over 2 years ago

  • Sprint/Milestone set to 3.0.0

3.0 version split out. Why are the changes to dependencies/tolua-5.2/src/Makefile.am needed in 3.0?

#5 Updated by Zoltán Žarkov over 2 years ago

tolua does not make an executable binary that can be used to compile pkg files in the emscripten build, so it requires system tolua in PATH. The tolua Makefile.am changes reflect not building the tolua binary.

#6 Updated by Sveinung Kvilhaugsvik over 2 years ago

3.0 issue: fcmanual=yes should be fcmanual=wiki

I had a very quick look at the Qt patch. Why add -O3 to CXXFLAGS there but not here?

#7 Updated by Sveinung Kvilhaugsvik over 2 years ago

3.0 version split out.

for real this time

#8 Updated by Zoltán Žarkov over 2 years ago

I had a very quick look at the Qt patch. Why add -O3 to CXXFLAGS there but not here?

You can add that to CXXFLAGS here, but I did not put them in the original patch because the SDL2 client does not build any C++.

#9 Updated by Zoltán Žarkov over 2 years ago

Do you need help on this to get it ready to commit?

#10 Updated by Sveinung Kvilhaugsvik over 2 years ago

  • Assignee changed from Sveinung Kvilhaugsvik to Zoltán Žarkov

Zoltán Žarkov wrote:

Do you need help on this to get it ready to commit?

Yes. Sorry if I gave the wrong impression. Assigning to you.

I believe there has been a misunderstanding. I was only planning to review this. I split it because I wanted a smaller, self contained part to make it easier for me to review it.

The self contained part I aim at is "can compile at least /common/". Building libfreeciv or something like it (Feature #847678) is not required here.

You can add that to CXXFLAGS here, but I did not put them in the original patch because the SDL2 client does not build any C++.

The setting of the CXXFLAGS fragment it modifies comes from Feature #838867. I probably included to much of Feature #838867 when I split this out. You are the expert here. If other parts of the patch fragment is needed (or recommended) to properly compile /common/ my suggestion is that the patch fragment stays but that the -O3 change is added already here.

The tolua Makefile.am changes reflect not building the tolua binary.

Doesn't

if test "x$cross_compiling" = "xyes" || test "x$emscripten" = "xyes"; then
sys_tolua_cmd=true

handle that?

#11 Updated by Sveinung Kvilhaugsvik over 2 years ago

Sveinung Kvilhaugsvik wrote:

Building libfreeciv or something like it (Feature #847678) is not required here.

Just to be 100% clear: I don't expect you to do Feature #847678

#12 Updated by Marko Lindqvist over 2 years ago

  • Sprint/Milestone changed from 3.0.0 to 3.1.0

To me this patch seems to be rather dirty. It might be good for local use (so you got the emscripten client already to use), but it's not ready to go in mainline. I may split this even further, to reconstruct it piece by piece. Also, I'm happy if we find time to do this for 3.1, I don't plan to port the patches to S3_0 (but we can of course reconsider that once we have working system ready).

#13 Updated by Marko Lindqvist over 2 years ago

#14 Updated by Marko Lindqvist over 2 years ago

  • Related to Feature #869884: emscripten: Disable zlib configure check added

#15 Updated by Marko Lindqvist over 2 years ago

#16 Updated by Marko Lindqvist over 2 years ago

#17 Updated by Marko Lindqvist over 2 years ago

  • Related to Feature #870642: emscripten: Disable icu configure check added

#18 Updated by Marko Lindqvist over 2 years ago

Build configured as 'CC=emcc CXX=em++ AR=emar --disable-sys-lua --disable-client' now goes through. I think that satisfies the minimum target for this ticket. Ok to close?

#19 Updated by Sveinung Kvilhaugsvik over 2 years ago

Marko Lindqvist wrote:

Build configured as 'CC=emcc CXX=em++ AR=emar --disable-sys-lua --disable-client' now goes through. I think that satisfies the minimum target for this ticket. Ok to close?

Yes.

#20 Updated by Marko Lindqvist over 2 years ago

  • Status changed from In Progress to Closed
  • Assignee changed from Zoltán Žarkov to Marko Lindqvist

Also available in: Atom PDF