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

Fix build with glib2 2.67.3+ #912

Merged
merged 1 commit into from Feb 15, 2021
Merged

Fix build with glib2 2.67.3+ #912

merged 1 commit into from Feb 15, 2021

Conversation

wenottingham
Copy link
Contributor

glib headers should not be included with 'extern "C"'.

Change in glib that caused this: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1715

Discussion of whether to work around it in glib: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1935. This was rejected, the suggestion there was to fix it in the including programs.

It may be possible for an expert in the header inclusion hierarchy to make a more targeted fix - I just kept editing files until it built.

Build tested on Fedora rawhide, smoke tested by starting the program with a new account from scratch.

glib headers should not be included with 'extern "C"'.
@code-gnucash-org code-gnucash-org merged commit f60b635 into Gnucash:master Feb 15, 2021
@jralls
Copy link
Member

jralls commented Feb 15, 2021

Thanks! I didn't even see this one coming.

BTW it's not possible to do a more targeted fix because glib won't let you include most internal headers from outside. You have to import glib.h. There are a few exceptions that add special features like glib/gprintf.h and glib/gi18n.h but neither of those has anything to do with this.

One other thing: It's better to put #include <glib.h> before the extern "C" block to override includes inside other headers declared extern "C".

@christopherlam
Copy link
Contributor

Shouldnt this go to maint?

@jralls
Copy link
Member

jralls commented Feb 16, 2021

Yeah, the new glib will leak into something--likely MinGW64--before 5.0 gets released.

@wenottingham
Copy link
Contributor Author

It's already in Fedora rawhide (which is how I ran across it.)

@jralls
Copy link
Member

jralls commented Feb 16, 2021

Does that mean that the next Fedora release will include it along with a recent GnuCash release (4.4 or 4.5 I suppose)? If so, why did you branch on master instead of maint?

@wenottingham
Copy link
Contributor Author

Because I goofed up the branch, no other reason. Would you like another PR, or is that merge something you can do?

@jralls
Copy link
Member

jralls commented Feb 16, 2021

Easier for me to do it.

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