Magnum hunter #298

pthom · 2018-12-03T10:57:54Z

Hi,
As discussed, here is the PR for the hunter related modifications.
Cheers

…Lists package/hunter ├── .gitignore # ignore HunterGate.cmake ├── HunterFetchDeps.cmake # Fetch SDL, corrade and glfw ├── HunterGate.cmake # main hunter scripts (ignored in git) └── HunterInit.cmake # entry point

codecov-io · 2018-12-03T12:29:44Z

Codecov Report

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

@@           Coverage Diff           @@
##           master     #298   +/-   ##
=======================================
  Coverage   52.89%   52.89%           
=======================================
  Files         323      323           
  Lines       16583    16583           
=======================================
  Hits         8771     8771           
  Misses       7812     7812

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 01a64a6...1af4ef0. Read the comment docs.

mosra

Thank you!

To save your time, I tried to find answers to my questions by reading Hunter docs. It was overwhelming but I think I got most of the answers from there. I'm still not sure about one thing, see below.

CMakeLists.txt

package/hunter/HunterInit.cmake

pthom · 2018-12-03T15:11:22Z

Hi, some updates about hunter CI:

Corrade Hunter CI

We will soon have green lights on corrade in the CI

https://travis-ci.org/pthom/hunter/builds/462832221
https://ci.appveyor.com/project/pthom/hunter
(I gave up on msys and mingw, as this requires to upgrade the compiler)

Magnum Hunter CI

I rebased all my code onto your last commits

https://travis-ci.org/pthom/hunter/builds/462840758
https://ci.appveyor.com/project/pthom/hunter/builds/20731321
(I gave up on msys and mingw, as they do not work in corrade)
I did not solve the issue with iOS ([hunter ** FATAL ERROR **] 'find_package(EGL)' should be called with REQUIRED). I have read to many logs, my eyes are bleeding :-)
Concerning this build, I had to upgrade the CI from ubuntu 14 to 16 (and I had to add libgl1-mesa-dev in order to have opengl)

mosra · 2018-12-03T15:51:07Z

Re the FATAL_ERROR about find_package(EGL): I'm aware of these issues and after reading Hunter docs I'm certain this needs a bigger update on my side. Putting that on my TODO list. I think not having an iOS build in the initial release is okay.

Re Ubuntu 14 vs 16: it's about this, right? https://travis-ci.org/pthom/hunter/jobs/462782227#L1290
I managed to work around this error for GCC builds, but not for Clang builds. This is because std::string move constructor is mistakenly not noexcept in libstdc++ < 5.5 but it's impossible to know what libstdc++ version is actually used.

I'll be switching to Ubuntu 16 soon anyway, so unless this is a critical issue for Hunter, I'm not sure we should bother too much with it.

pthom · 2018-12-03T15:55:22Z

Re the FATAL_ERROR about find_package(EGL): I think not having an iOS build in the initial release is okay.

I think too !

Re Ubuntu 14 vs 16: it's about this, right? https://travis-ci.org/pthom/hunter/jobs/462782227#L1290
I managed to work around this error for GCC builds, but not for Clang builds. This is because std::string move constructor is mistakenly not noexcept in libstdc++ < 5.5 but it's impossible to know what libstdc++ version is actually used.

Exactly, the error message is strange, given that the corrade code seems to do what is appropriate

I'll be switching to Ubuntu 16 soon anyway, so unless this is a critical issue for Hunter, I'm not sure we should bother too much with it.

I hope the maintainer of hunter will bare with me for I changed from travis to xenial. Having a package like magnum is probably worth it !

…if needed

mosra · 2018-12-03T16:09:07Z

I just pushed mosra/corrade@c09a22e which is hopefully a better workaround for the noexcept issue and could make build on 14.04 possible again. Not on master yet, waiting for the build to succeed but looks good so far. Could you try on the CI with this commit and 14.04?

EDIT: it's on master now.

pthom · 2018-12-03T16:27:46Z

Hum, it would be too long with travis.
However I have good news, with a test from docker :

If I use ubuntu 14.04

diff --git a/Docker/ubuntu/Dockerfile b/Docker/ubuntu/Dockerfile
index abc754c..eecd979 100644
--- a/Docker/ubuntu/Dockerfile
+++ b/Docker/ubuntu/Dockerfile
@@ -1,7 +1,7 @@
-FROM ubuntu:16.04
+FROM ubuntu:14.04

And then inside the docker container I try to build corrade I have the error:

> ./TLDR_hunter.py test-build corrade gcc-7-cxx14
/sources_docker/corrade/src/Corrade/Utility/FileWatcher.cpp:59:14: error: function ‘Corrade::Utility::FileWatcher& Corrade::Utility::FileWatcher::operator=(Corrade::Utility::FileWatcher&&)’ defaulted on its redeclaration with an exception-specification that differs from the implicit exception-specification ‘’
 FileWatcher& FileWatcher::operator=(FileWatcher&&)

then I fetch your modif

> cd corrade
> git pull upstream ae1fc17d3d77b1bec248382fbdbefd8b9fbb44a9

And inside the container, it works !

> ./TLDR_hunter.py test-build corrade gcc-7-cxx14
SUCCESS

What is your opinion: do you prefer that I reverse the travis builds to travis ?

mosra · 2018-12-03T16:28:34Z

Awesome! :) It's on master now.

pthom · 2018-12-03T16:42:18Z

mosra · 2018-12-03T17:13:20Z

Hunter CI Back to travis ?

Not really sure what this means, sorry 😅

Create a release

I'm planning to have a 2018.12 release, but that will happen later this month (around Dec 30th I think) as I still have quite a few important features I want to finish before tagging another version. Would it be possible to use a temporary tag on your fork in the meantime? Once 2018.12 is out, I can switch the packages to the "upstream" tag.

Integrate example_apps into magnum-bootstrap ?

Yes, definitely. I can do that once Magnum is available in Hunter, I also wanted to play with it a bit to get a feeling how it's used. For the rest of the magnum_hunter I'm not quite sure yet, maybe it could go also to package/hunter/ to aid with packaging and testing in the future. Would it be okay to just keep it on your account until I have a clearer idea?

pthom · 2018-12-03T17:43:13Z

Hunter CI Back to travis ?

I need to discuss this directly with Ruslan Baratov (maintainer of hunter)

Create a release: I'm planning to have a 2018.12 release, but that will happen later this month (around Dec 30th I think) as I still have quite a few important features I want to finish before tagging another version

There is no hurry.

Would it be possible to use a temporary tag on your fork in the meantime?

Not on my fork. If we want to publish a release before the 2018.12 release, I would need to place my fork inside https://github.com/hunter-packages/ (which I did not).
But if I copy the fork there, it should be ok, and I can create a tag there.

For the rest of the magnum_hunter I'm not quite sure yet, maybe it could go also to package/hunter/ to aid with packaging and testing in the future. Would it be okay to just keep it on your account until I have a clearer idea?

Of course, this is the good way to go.

pthom · 2018-12-03T17:46:47Z

One question that is abolutely not related.
I am quite new to magnum actually. When I build the primitive example on my mac (https://github.com/mosra/magnum-examples/tree/master/src/primitives) , I get a black screen with the following output:

Platform::Sdl2Application: warning: the executable is not a HiDPI-enabled app bundle
Renderer: AMD Radeon Pro 560 OpenGL Engine by ATI Technologies Inc.
OpenGL version: 4.1 ATI-2.2.8
Using optional features:
    GL_ARB_ES2_compatibility
    GL_ARB_separate_shader_objects
    GL_ARB_texture_filter_anisotropic
    GL_ARB_texture_storage
    GL_ARB_vertex_array_object
    GL_EXT_debug_label
    GL_EXT_debug_marker
Using driver workarounds:
    no-layout-qualifiers-on-old-glsl

Did I miss a setting?

It does work on windows, though.

mosra · 2018-12-03T17:52:44Z

No, you didn't. it's a known bug in macOS 10.14. Shake the window, minimize/maximize it or click into it to make the contents appear. Related info for SDL is here: https://discourse.libsdl.org/t/macos-10-14-mojave-issues/25060 (so far I'm not aware of a new version fixing this :/ ...), more info (and some workarounds) here in a GLFW issue: glfw/glfw#1334

pthom · 2018-12-03T18:01:25Z

Shake the window, minimize/maximize it or click into it to make the contents appear

This is the funniest workaround I have heard off :-)

It does not work on my side because I cannot resize the example app window. It is not important at all, though: I can work on linux / windows.

mosra · 2018-12-03T18:08:56Z

Yes, it's quite annoying. Not related to Magnum at all, but rather to how the underlying toolkits (SDL/GLFW) manage the GL context. I hope the fixed versions of SDL / GLFW get released soon.

For the release tag: so is it okay to have the release delayed until I get 2018.12 out? I really don't want to be blocking you from getting your work done :)

Oh, but Corrade (2018.10) could get packaged as-is already so we can tick at least some boxes, right? Then, once that's done, the hunter URL here could get updated to an upstream version of Hunter that contains Corrade and then this PR could get merged without any TODOs left. Is that a reasonable path forward?

pthom · 2018-12-03T18:57:08Z

Ok, let's try to tick some boxes for corrade :

Here is the first PR out of two : ingenue/hunter#300
(this one contains the CI scripts)

CMakeLists.txt

package/hunter/HunterInit.cmake

… check signature

pthom · 2018-12-05T09:03:20Z

Hi,
I just pushed some modifs to ingenue/hunter#300 (review)

This version should pass, as it disables the hack for corrade-rc.

For the future release of corrade on hunter, we have two options :

Leave the p.pkg.corrade branch as it is, i.e do not build iOS and Android packages in hunter's CI (I added a note inside the readme that explains it can work if corrade-rc is in the path)
Add some scripts into the corrade repo : something that will cause corrade-rc to try to build automatically if find_program(corrade-rc) fails (this might be based on a cmake flag) . I do not think it is worth adding a dependency to hunter for this, as corrade does not require hunter (it has no dependency), so that a shell script called by cmake could be sufficient.

Do you have a preference?

mosra · 2018-12-05T11:37:39Z

@pthom thank you!

I would leave it as-is, for now. With other cross-compilation packages I'm usually adding a dependency to native Corrade installation (so e.g. ArchLinux emscripten-corrade depends on native corrade). Not sure if anything like that can be expressed in Hunter, but again, let's leave it for a later time :)

ruslo · 2018-12-05T11:49:40Z

I do not think it is worth adding a dependency to hunter for this, as corrade does not require hunter (it has no dependency), so that a shell script called by cmake could be sufficient

Target version of package depends on host build of same package. So there is a dependency.

Not sure if anything like that can be expressed in Hunter

Yes, it can be, see comments:

mosra · 2018-12-05T12:04:41Z

@ruslo right, thanks, sorry that you had to repeat everything again 😅

Yes, now I think I know how this could be done (as a next step after the native packages are done). Since I still had some problems with e.g. find_package() not having REQUIRED clauses for Android / iOS and so I need to fix that first.

pthom · 2018-12-05T19:11:46Z

corrade is merged into hunter !
ruslo/hunter#1646

mosra · 2018-12-05T19:16:01Z

Great!

I'll reference it in the building documentation. Is there anything I should mention besides "Corrade is available in Hunter" and linking to https://docs.hunter.sh/en/latest/packages/pkg/corrade.html ?

Can you update the Hunter release URL here? I'll merge this PR right after.

ruslo · 2018-12-05T19:26:27Z

I'll reference it in the building documentation. Is there anything I should mention besides "Corrade is available in Hunter" and linking to https://docs.hunter.sh/en/latest/packages/pkg/corrade.html ?

I think just link should be enough, user have to read basic Hunter documentation anyway. By the way you can add it as a badge: https://docs.hunter.sh/en/latest/creating-new/create/cmake.html#badge

mosra · 2018-12-05T19:49:33Z

Yay, badge! Of course :)

Done in mosra/corrade@a4c4fe6, updated docs here.

mosra · 2018-12-11T19:31:13Z

Merged a squashed version in 91b41a7, I did a bit of simplification and coding style updates on top.

@pthom once I tag 2018.12, I'll let you know, and in case you are not around, I'll attempt to finish the Magnum package submit myself. Thanks a lot for the contribution! 👍

pthom · 2018-12-12T21:25:07Z

@mosra : thanks for the update!

Please do contact me when you release the version, I'll try to help!

pthom added 4 commits December 3, 2018 11:19

package/hunter/HunterInit.cmake : add fixme warning

a98873a

HunterFetchDeps : if (WITH_AUDIO) => hunter_add_package(OpenAL)

ec92c24

package/hunter/HunterInit.cmake : use hunter_rc3 release

2f9046e

package/hunter/HunterInit.cmake : wrong url

683a81a

mosra reviewed Dec 3, 2018

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

package/hunter/HunterInit.cmake Show resolved Hide resolved

package/hunter/HunterInit.cmake Outdated Show resolved Hide resolved

package/hunter/HunterInit : download HunterGate from pinned version, …

7ac3291

…if needed

mosra mentioned this pull request Dec 3, 2018

Fix compil error in GlyphCache.cpp + hunter #297

Closed

ruslo reviewed Dec 3, 2018

View reviewed changes

CMakeLists.txt Show resolved Hide resolved

CMakeLists.txt Show resolved Hide resolved

package/hunter/HunterInit.cmake Outdated Show resolved Hide resolved

HunterInit.cmake : download gate to CMAKE_BINARY_DIR / check status /…

1af4ef0

… check signature

pthom force-pushed the magnum_hunter branch from 43842a0 to 1af4ef0 Compare December 3, 2018 21:21

mosra added this to the 2018.1d milestone Dec 11, 2018

mosra closed this Dec 11, 2018

mosra mentioned this pull request Dec 29, 2018

2019.01 #302

Closed

55 tasks

mosra mentioned this pull request Jan 5, 2019

Conan package #304

Closed

mosra added the changelog mention added label Jan 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Magnum hunter #298

Magnum hunter #298

pthom commented Dec 3, 2018

codecov-io commented Dec 3, 2018 •

edited

Loading

mosra left a comment

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018 •

edited

Loading

pthom commented Dec 3, 2018 •

edited

Loading

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018 •

edited

Loading

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

pthom commented Dec 5, 2018 •

edited

Loading

mosra commented Dec 5, 2018

ruslo commented Dec 5, 2018

mosra commented Dec 5, 2018

pthom commented Dec 5, 2018

mosra commented Dec 5, 2018

ruslo commented Dec 5, 2018

mosra commented Dec 5, 2018

mosra commented Dec 11, 2018

pthom commented Dec 12, 2018

Magnum hunter #298

Magnum hunter #298

Conversation

pthom commented Dec 3, 2018

codecov-io commented Dec 3, 2018 • edited Loading

Codecov Report

mosra left a comment

Choose a reason for hiding this comment

pthom commented Dec 3, 2018

Corrade Hunter CI

Magnum Hunter CI

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018 • edited Loading

pthom commented Dec 3, 2018 • edited Loading

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018 • edited Loading

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

mosra commented Dec 3, 2018

pthom commented Dec 3, 2018

pthom commented Dec 5, 2018 • edited Loading

mosra commented Dec 5, 2018

ruslo commented Dec 5, 2018

mosra commented Dec 5, 2018

pthom commented Dec 5, 2018

mosra commented Dec 5, 2018

ruslo commented Dec 5, 2018

mosra commented Dec 5, 2018

mosra commented Dec 11, 2018

pthom commented Dec 12, 2018

codecov-io commented Dec 3, 2018 •

edited

Loading

mosra commented Dec 3, 2018 •

edited

Loading

pthom commented Dec 3, 2018 •

edited

Loading

pthom commented Dec 3, 2018 •

edited

Loading

pthom commented Dec 5, 2018 •

edited

Loading