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

Rebase patches for fixing haiku build. #20913

Merged
merged 1 commit into from
Aug 13, 2018
Merged

Conversation

pulkomandy
Copy link
Contributor

As requested in #18310.

@pulkomandy pulkomandy requested a review from a team as a code owner August 11, 2018 11:06
@pulkomandy
Copy link
Contributor Author

pulkomandy commented Aug 11, 2018

I have a problem: clang-format complains that the includes are not ordered alphabetically. This is on purpose, kernel/image.h must be included before other system include files, because they include image.h and end up getting the one from godot instead of the one from system directories.

Ideally, Godot would not add its own directories to system include directories, but only to local ones (using -iquote instead of -I for gcc) to avoid the problem. I'm not familiar enough with your buildsystem to do this, however.

@Faless
Copy link
Collaborator

Faless commented Aug 11, 2018

@pulkomandy
Put this wherever you want, it will disable clang-formatting between the comments on/off. :)

// This must always be included first, disabling clang format
/* clang-format off */
#include <kernel/image.h> // needed for image_id
/* clang-format on */

@fire
Copy link
Member

fire commented Aug 11, 2018

Alternatively leave a space between the entries.

@pulkomandy
Copy link
Contributor Author

There was a build failure but it looks unrelated. What happened?

@@ -77,7 +77,7 @@ void close_gl(void) {
#include <dlfcn.h>
static void* libGL;

#ifndef __APPLE__
#if !defined(__APPLE__) && !defined(__HAIKU__)
Copy link
Member

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

Will be merged upstream, I'll get to it tomorrow (maintainer of glad here).

@@ -44,6 +44,9 @@
#elif defined POSH_OS_FREEBSD
# define NV_OS_FREEBSD 1
# define NV_OS_UNIX 1
#elif defined POSH_OS_HAIKU
Copy link
Member

Choose a reason for hiding this comment

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

Same here, it would be great if you can try to get those changes upstreamed at https://github.com/Thekla/thekla_atlas.

@akien-mga akien-mga added this to the 3.1 milestone Aug 13, 2018
@akien-mga
Copy link
Member

Thanks a lot! Looks good to me.

@akien-mga akien-mga merged commit 139e621 into godotengine:master Aug 13, 2018
@pulkomandy
Copy link
Contributor Author

Upstream PRs submitted:
Dav1dde/glad#170
Thekla/thekla_atlas#22

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

Successfully merging this pull request may close these issues.

5 participants