Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update C++ version to support Node 16 on Mac #215

Merged
merged 1 commit into from
Aug 18, 2021

Conversation

zakjan
Copy link

@zakjan zakjan commented Jun 7, 2021

Compiling on Node 16 currently fails with:

/Users/zakjan/Library/Caches/node-gyp/16.2.0/include/node/v8-internal.h:452:38: error: no template named 'remove_cv_t' in namespace 'std'; did you mean 'remove_cv‘?

I tracked it down to that std::remove_cv_t was added in C++14 https://en.cppreference.com/w/cpp/types/remove_cv while headless-gl compilation forces C++11 on Mac https://github.com/stackgl/headless-gl/blob/master/binding.gyp#L44

After updating C++ version, it compiles well. Running tests, I get a test failure which was already reported as #171, so I suppose it's unrelated.

@zakjan
Copy link
Author

zakjan commented Jun 7, 2021

Travis check fails because make images timeouts. I'm not sure what causes it?

@dhritzkiv
Copy link
Member

Not sure how I missed this PR. Apologies!

LGTM.

I'll investigate Travis separately.

@dhritzkiv dhritzkiv merged commit 9055931 into stackgl:master Aug 18, 2021
@zakjan
Copy link
Author

zakjan commented Aug 19, 2021

@dhritzkiv Thanks for merging! Could you also release it? It's going to help with development of downstream libraries such as luma.gl

@dhritzkiv
Copy link
Member

I'm been trying to on-and-off on my breaks today, but am running into all of the issues with Travis.

It seems that node 16 changed some of the requirements for the compiler, which changed the requirements for which linux distro we should use in CI (we were still on Trusty), which changed how the gl libraries behave, which cause the tests to fail, which keeps us from building prebuilt binaries for release. And that's just the linux tests.

I'll try again tomorrow.

@zakjan
Copy link
Author

zakjan commented Aug 19, 2021

🤯 ok, thanks for the info

@dhritzkiv
Copy link
Member

Sometimes OSS maintenance is fun and rewarding… other times, its spent updating dependencies 😄

@himself65
Copy link

When does the new version publish?

@dhritzkiv
Copy link
Member

Sorry everyone, the CI was giving me lots of trouble with tests, and without CI, I couldn't produce a release. This was further compounded by running out of open source credits to use towards CI, so I couldn't test as frequently.

This fix is now released in v4.9.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants