Schala commented on 2017-08-09 22:07

Martchus commented on 2017-08-08 15:48

@jbg I obviously didn't have that issue last time I built this package (December last year). Maybe a change in the behaviour of depot-tools/ninja.

General note: I'll update mingw-w64-angleproject again when updating mingw-w64-qt5-webkit to 'ng' version. Then I'll drop the patches to make WebKit use this package which will allow to get rid of `0005-Export-shader-API-via-libGLESv2.dll.patch`. The idea of using system ANGLE rather than the bundled version for WebKit from the previous maintainer was nice, but it has become too hard to maintain.

jbg commented on 2017-08-08 08:51

To make this package build, I had to change `--depth .` to `--depth ..` in gyp_args and `out/Release` to `../out/Release` in ninja_args, in the build function in PKGBUILD.

Martchus commented on 2016-11-06 19:23

I've just pushed a new commit. Added the flags for optimization and updated the to the latest version. However, I haven't tested whether it actually runs.

This is using now dept-tools and ninja (seems to be the recommend way of building ANGLE). Concurrent builds are now not broken anymore, so this should build much faster. I hope I haven't broken shader API required by Qt WebKit.

Martchus commented on 2016-08-11 18:35

As I already said, the flags are taken from Fedora. The Fedora devs chose the flags so they match as best as possible the default flags for regular packages under Fedora (according to some guy at #fedora-mingw IRC channel). I agree with that strategy. Hence, here we should use flags which match best the default flags under Arch.

So the default flags under Arch indeed include the proposed flag. I think that would be a good reason to also include the flags here.

However, for consistency that decision should be done for all mingw-w64 packages and not only for this one. Till a general decision is made I will not update the flags. And there are more things to consider.

I should also mention that the current version of mingw-w64 has some serious bugs. The i686 version of binutils is more or less unusable (just have a look at the bug reports concerning the mingw-w64-binutils package).
There are also bugs triggered by optimization. For example if you instantiate std::stringstream inside a library built with at least -O1 the application crashes with a page fault. (I already filed a bug report bug haven't got any response so far: https://sourceforge.net/p/mingw-w64/bugs/548/.) I've just took a look and noticed that ANLGE also makes use of that class so if you experience crashes that might be the reason. (That means using the flags currently present in the PKGBUILD is actually a bad idea with the current version of mingw-w64!)

So you see that adding flags (especially concerning optimizations) might has side affect such as very annoying bugs. Hence I suppose it is best to wait for a more stable release of mingw-w64 before changing the flags for mingw-w64 packages. Some testing would also be required.

BTW: I recently tried to rebuild this using a newer commit ID and it failed. Not even because of the patches but because of missing headers provided by another Google project. I guess it would be better if this PKGBUILD would use depot_tools and maybe ninja according to the build instructions.

b00rt00s commented on 2016-08-09 21:41

Of course the package builds correctly, but it's not optimized, I think. The flags could be added for the sake of performance. I must, however, admit that I'm not expert here (just guessing).

Martchus commented on 2016-08-09 16:18

Hi,

of course would it be possible to add the flag. However, what would be the benefit? As far as I see it the package builds correctly without it.

Martchus commented on 2016-05-18 21:57

Gusar commented on 2016-03-16 20:49

Yes, everything from previous build attempts is deleted.

About angle devs, there's already an upstream bug report [1], hopefully there will be some movement there, but I kinda have my doubts. As it is, it seems I have no other option but to build myself with modified headers. Maybe I'll play around further in the coming days, we'll see.

Martchus commented on 2016-03-16 20:14

I can not change the headers as you proposed because this would likely break dynamic linkage. Since there is currently no other solution I don't update the package for now.

However, you could try to ask the ANGLE devs. They already gave me the hint how to build the static libs at all but noted that this is not officially supported.

When you tried to add the define before building the static ANGLE libs, did you ensure that all relevant object files from the previous shared build are deleted? It might haven been the case that these just haven't been rebuilt. That's currently my last idea.

Gusar commented on 2016-03-16 17:44

> However, I'm wondering how you managed to build statically without getting all those errors I got.

Simple - I don't build with the kitchen sink included. In serious words, I do a minimal build with just libdvdcss, libdvdread, libdvdnav, zlib, freetype2, libass, luajit, ffmpeg (absolutely no external libs here, just ffmpeg's internals - libavcodec and company). All of them compiled manually, mainly because I compiled them before I knew there's a whole bunch of mingw packages in AUR.

I think I know where your errors come from - just like I had to change lib='EGL' to a whole list of libraries for static angle, you'll have to do similar for all the projects that fail linking for you. The way I see it, when you link statically, you need to tell the linker where to get all the objects that a dynamic library will resolve by itself at runtime.

You are right, some libs are still linked dynamically (EGL is among them). I think mpv devs to this for the simple reason that static builds are not officially supported by ANGLE. However, I'm wondering how you managed to build statically without getting all those errors I got.

Gusar commented on 2016-03-16 14:37

I use sort of a hack to build statically - I remove the import libs (.dll.a). There's the --enable-static-build switch, but it doesn't work for everything. Then, you need to specify all libraries that will get linked in - in mpv's wscript, find the egl-angle section and change lib='EGL' to lib=['EGL', 'GLESv2', 'ANGLE', 'angle_common', 'translator', 'translator_lib', 'preprocessor', 'dxguid', 'd3d9', 'gdi32', 'stdc++']

As for verbosity, there's build/config.log, it contains basically everything you'd want to know.

Martchus commented on 2016-03-15 21:22

Ok, so adding those definitions doesn't resolve the problem.

I just tried to build mpv myself to reproduce the issue and it compiled - however it "just" linked dynamically by default. Then tried to link statically to be able to reproduce the error, but I'm unable to convince waf to link statically. How is that achieved? Which commands do you use to build mpv? Is there a way to make waf more verbose? Currently I can't see which linker flags are actually used.

When using my repository, be aware that is might not be always available since it is just home hosted.

Gusar commented on 2016-03-15 15:00

Yes, I'm referring to your binary repository (my goal is to be able to use that, instead of needing to compile angle myself). The success I had was when I built my own package with that sed line added to the PKGBUILD. Your package, built without that, doesn't work. I should've mentioned that, sorry for the confusion.

Settings those defines just for mpv doesn't work (same error). So they'll need to be set also when building the static libs. Using CFLAGS/CXXFLAGS doesn't get them picked up by gyp, while it does work with waf. Not that I liked it before, but I'm beginning to actively dislike gyp. I'll do a search about gyp to see if it's possible to add preprocessor defines from the command-line. Note that I had to set the flags like so: CFLAGS="-DGL_APICALL= -DEGLAPI=" (the '=' is important)

EDIT: I added -DGL_APICALL= -DEGLAPI= to where the PKGBUILD already sets CXXFLAGS. Looking carefully at the make output, they're there. But the resulting libEGL.a still contains for example _imp___ZN3egl14GetProcAddress instead of ZN3egl14GetProcAddress

Martchus commented on 2016-03-15 13:58

"With these new libs I was able to cross-compile mpv.exe with Angle statically built in."
So the package works in general (with the modifications you mentioned) but the binary package in my repository doesn't? Or have you just found out that it doesn't work after all?

"So I guess the next step is convincing gyp to add the defines when building the static libs, and then convincing waf (mpv's build system) to add them."
I think adding additional definitions can be achieved by simply setting CFLAGS+='-DGL_APICALL -DEGLAPI' CXXFLAGS+='-DGL_APICALL -DEGLAPI'. This way I also added additional include directories. This approach should work for all (meta) build systems which actually just generate a Makefile (cmake, qmake, gyp, ...). I don't know whether it works for waf.

These definitions will prevent that the compiler assumes stdcall decorated symbols are present. Your linker errors indicate that the linker is looking for stdcall decorated symbols but the library only contains regular symbols. If this conjecture is true, you should be able to link if you add the definitions as described when building mpv but without the need to modify/rebuild this package. Additionally, if this conjecture is true this problem should only occur when building for i686 but not when building for x86_64.
Unfortunately my experience with static linking and the name mangling behaviour under MS Windows is limited so that's only my best guess.

Gusar commented on 2016-03-14 21:36

I couldn't successfully statically link using your binary package, I get [1]. According to [2], one needs to mangle the headers or do -DGL_APICALL -DEGLAPI both when compiling the static Angle libs and when compiling the application (mpv in my case).

So I guess the next step is convincing gyp to add the defines when building the static libs, and then convincing waf (mpv's build system) to add them. Tomorrow though, it's bed time for me now.

Martchus commented on 2016-03-14 18:03

Since the usage of __declspec/__attribute__ is controlled according particular macro definitions, it shouldn't be hard to pick the right one by just defining/undefining relevant definitions before including the header. This way no further manipulation of the files is required.

Gusar commented on 2016-03-14 14:52

Thanks for fixing the static libs. Though I forgot a little hack is needed to get them usable, a proper fix will need to be done by upstream. In the current code, all the __declspec stuff needs to be removed from the header files, otherwise static linking fails. Some sed is useful here:

I'm quite sure this will break dynamic linking, so it's not something you can put into the PKGBUILD. But as it's headers, I can do it locally after installing the mingw-w64-angleproject binary package.

Martchus commented on 2016-03-13 18:31

Since I usually don't link statically I didn't test them and I didn't really know what these thin archives are. Thanks for making this clear and providing the patch.

"Angle is a real pain to build,"
In fact the current problems are:
- mingw-w64 lacks some required header files.
- The gyp Makefile generator seems broken. At least concurrent builds don't work for me.
- I'm unable to enable the ninja generator, so this workaround is not possible, too.
- Generation of import libs and use of *.def files must be patched.
- 32-bit *.def files are not provided by ANGLE.
- Shader symbols must be exported manually by patching the project file to be able to build Qt WebKit which must be patched as well because ANGLE devs decided to change the API.
- Building static libs is not officially supported but seems to work anyways (thanks again for your patch).

Gusar commented on 2016-03-13 15:20

A note on static libraries: "Thin archives" merely reference object files, they don't contain any objects themselves. Which means one needs the original object files around, something this package doesn't provide. So it's not possible to use this package to statically build Angle into a binary, the thin archives by themselves are useless. I suggest turning them into actual static libraries. Googling around, I found some code to do that, modified for this PKGBUILD it looks like this (add it after the static libs are built):

What the code does: "ar -t" lists the object files referenced by the thin archive, these object files are then packed into a new library, which then replaces the thin archive. Finally, ranlib is needed to add an index to the new library. With these new libs I was able to cross-compile mpv.exe with Angle statically built in.

Thanks for this package BTW. Angle is a real pain to build, I don't think I'd get anywhere if it weren't for your work.

Martchus commented on 2016-03-10 19:30

Just rebuild this, if you want this to work with Qt WebKit.

I included a patch to export symbols which are required to use the C++ interface declared in ShaderLang.h via libGLESv2.dll. This interface is used in Qt WebKit.

I hope I didn't break anything further in the build system because I had to change a few details. The units providing the interface are now build as part of the libGLESv2 target which now no longer depends on an extra static library providing the interface.

Martchus commented on 2016-02-27 21:04

I'll add the -f flag. I wanted to update the package for other small changes anyways. However, I can't reproduce the issue - rm doesn't ask me when building this package.

I know what the guideline says. When adding -git suffix I would remove the commit ID of course. But you're right, for this package it is better to stick with a particular commit for a while.

Thanks for providing me some sources but I've already checked them. These repositories are helpful but not completely appropriate:
Qt 5/Fedora: They include a lot of patches. Including a lot of patches is not the Arch way. Especially since my Qt 5 applications (including Qt WebKit) seem run fine without them. Additionally Fedora sticks with a commit ID from 2014 which is quite old and a lot of patches don't apply anymore.
MSYS2: The current version of the package doesn't work. I already submitted a pull request (https://github.com/Alexpux/MINGW-packages/pull/1108).

ant32 commented on 2016-02-27 20:20

Could you change ge line to include 'f' so it doesn't ask the user for input during building?
rm -fr .git

As far as adding -git to the package don'thttps://wiki.archlinux.org/index.php/VCS_package_guidelines
says
Suffix pkgname with -cvs, -svn, -hg, -darcs, -bzr, -git etc. unless the package fetches a specific release.
Building the package always seems to require patches and hacks so it's good to have a package that won't break because of a git update. Hopefuly I'm wrong on the last statemnt.

Martchus commented on 2016-02-21 22:40

I adopted the package to fix the gyp dependency.

Since the current version is based on a snapshot from 2014 I also decided to update the package. However, I had to do a few adjustments to make it compile:
- I removed most of the patches since these can't be applied anymore. But it seems like all these patches aren't required anymore.
- I had to provide some header files which seem to be missing in mingw-w64-headers. I outsourced these headers to another repository because the max. upload size would be exceeded otherwise.
- I disable concurrent builds because building concurrently is broken.
- In the current version the static libs are created manually. As the selection of object files is no longer valid for the new version I decided to use "-D angle_gl_library_type=static_library" build flag to create the static libs with the build system.

I hope the package still works. At least my Qt 5 apps work with the new version without needing to recompile any Qt 5 packages.

As there are no official releases of the ANGLE project we might consider making this a *-git package? On the other side, compiling this might require some more adjustments in the future so sticking with a particular commit for a while isn't a bad idea either.

FreddieChopin commented on 2015-10-08 15:39

Just change the dependency in the PKGBUILD file from "gyp-svn" to "gyp-git" and it works.