glib can no longer be included in extern "C" blocks

Actually there is a better fix in Fedora: https://src.fedoraproject.org/rpms/gnucash/blob/7287f9ed18964fac061035ba3a86566e1a03c4c5/f/gnucash-glib.patch

Please cherry-pick it to get past the hirsute build failure: https://launchpadlibrarian.net/532491875/buildlog_ubuntu-hirsute-amd64.gnucash_1%3A4.4-1_BUILDING.txt.gz

The attachment "fix-build.patch" seems to be a debdiff. The ubuntu-sponsors team has been subscribed to the bug report so that they can review and hopefully sponsor the debdiff. If the attachment isn't a patch, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are member of the ~ubuntu-sponsors, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issue please contact him.]

Actually just submitted the cherry-pick'd Fedora patch to Debian:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=986969
https://salsa.debian.org/debian/gnucash/-/merge_requests/1

Looks like upstream has fixed this now with https://github.com/Gnucash/gnucash/commit/bbb4113a5a996dcd7bb3494e0be900b275b49a4f, so it would probably be best if we cherry-picked that commit. Could you prepare an upload with a quilt patch and dep3 headers against that upstream commit please, and I'll sponsor it? Or I can do it, but I don't want to steal your credit :)

This was just a case of not getting notified of my PR's activity and forgetting.. sorry bout that.

I've updated the PR to get the fix pulled in debian.

Thank you for working on getting this pulled into Debian!

What's your plan for Ubuntu? Are you holding on to see if Debian uploads soon so that Ubuntu can sync it? Or do you want your fix uploaded into Ubuntu ahead of Debian, pending it being fixed in Debian?

I would normally expect the latter for an FTBFS fix in Ubuntu, but if that's not your plan then we can leave it open. I'm just not sure what you intend because this bug still has ~ubuntu-sponsors subscribed but there's nothing ready for upload in Ubuntu right now, I don't think?

I've just put the patch on top of 4.4 that is in impish and uploaded the build artifacts to: https://launchpad.net/~hellsworth/+archive/ubuntu/gnucash

@Heather: Debian chose to fix it by updating to a new upstream release (version 4.6). Syncing into impish would require a FFe approval, I suppose:

https://github.com/Gnucash/gnucash/releases/tag/4.6

Better do that in next cycle IMO, and use your patch in Ubuntu for now.

However, the PPA build failed...

I digged deeper into it. The patch in your PPA actually just creates another patch, and doesn't change any upstream files. If you had done it right, you'd have found that it doesn't apply by itself.

I uploaded a fix. Besides the upstream commit which Robie mentioned, I included six other commits to make it work.

This bug was fixed in the package gnucash - 1:4.4-1ubuntu1

---------------
gnucash (1:4.4-1ubuntu1) impish; urgency=medium

  * Fix FTBFS due to glib headers in 'extern "C"' blocks (LP: #1923642)
    - No longer permitted with glib2.0 2.67.3+
    - The fix consists of these upstream commits:
      + Fix-build-with-glib2-2.67.x.patch
      + Move-glib-and-gtk-includes-out-of-extern-C-for-tests.patch
      + Potentially-fix-CI-test-on-Arch-related-to-glib-and-c++.patch
      + More-fixes-for-Arch-ci-failure.patch
      + Still-more-fixes-for-Arch-ci-failure.patch
      + Finish-the-glib-2.67-fixes-for-CI-tests.patch
      + Bug-798156-glib-2.68.0-breaks-gnucash.patch
    - Thanks to Heather Ellsworth for researching the issue!

 -- Gunnar Hjalmarsson <email address hidden> Wed, 08 Sep 2021 06:48:41 +0200

@Gunnar, Thanks! I didn't realize that the 4.6 that debian pulled fixes the issue. There was nothing in the 4.6 commits that suggested this issue had been fixed, so did you just build and test it to see? Or how did you know that 4.6 fixes the issue?

https://salsa.debian.org/debian/gnucash/-/compare/f9461670c5f516e31631a80975eebf6b6e56a1c6...c45dcd6db80fc45ef60cc5d30eb5612f6b64ce9d

I don't doubt you at all, just trying to figure out what I missed :)

Also, this creating of a patch worked 2 months ago to get past the build issue.. so yesterday I assumed it would still work. But it looks like there are more files to patch now and you've done that with your patches. But, why not put all of the patches into one big patch?

Keeping the patches separate is good practice in that they might not all be accepted upstream at the same time. Using this strategy its easier to drop individual patches, when merging with a new upstream version, then modifying one big patch.

On 2021-09-08 16:54, Heather Ellsworth wrote:
> @Gunnar, Thanks! I didn't realize that the 4.6 that debian pulled
> fixes the issue. There was nothing in the 4.6 commits that suggested
> this issue had been fixed, so did you just build and test it to see?
> Or how did you know that 4.6 fixes the issue?

First and foremost I noticed that gnucash 1:4.6-1 built on Debian with version 2.68 of glib2.0. Also, the "Bug 798156 - glib 2.68.0 breaks gnucash" commit predates the upstream 4.6 release. So I didn't build/test myself.

> https://salsa.debian.org/debian/gnucash/-/compare/f9461670c5f516e31631a80975eebf6b6e56a1c6...c45dcd6db80fc45ef60cc5d30eb5612f6b64ce9d

The master branch of the gnucash repo seems to only include the debian/ directory, not the upstream files.

> I don't doubt you at all, just trying to figure out what I missed :)

Sure, n.p.

> Also, this creating of a patch worked 2 months ago to get past the
> build issue..

Well, the "Bug 798156 - glib 2.68.0 breaks gnucash" patch never was sufficient on top of version 4.4 of gnucash. It didn't even apply without first applying a bunch of other commits. Probably it makes a difference on top of version 4.5, though.

What you put in your Debian MR seems to be the same as in your PPA. Neither of them changes anything in substance. So it appears you never built with that patch before submitting your MR. Compare your MR with the equivalent patch I included in the fix to understand what I'm talking about.

> But, why not put all of the patches into one big patch?

In addition to Brian's comment on that, I can say that the fix is the result of some trial and error. Concatenating the patches would have been additional work. :/

TBH I fear that there would have been a much simpler solution:

https://gitlab.gnome.org/GNOME/glib/-/issues/2331#note_1067322

But I saw that comment only when the fix was working, and now I'm disinclined to try it with the risk to find out that the work I did was wasted...

> Keeping the patches separate is good practice in that they might not all be accepted upstream at the same time.

Ah that makes sense!

> The master branch of the gnucash repo seems to only include the debian/ directory, not the upstream files.

This threw me off and that's why I made a patch that made a patch, thinking that's what would have had to happen. I see that while that needs to happen sometimes for some projects (libreoffice), it's not a hard rule.

> Well, the "Bug 798156 - glib 2.68.0 breaks gnucash" patch never was sufficient on top of version 4.4 of gnucash.

I could have sworn I tested it but looking back at my notes from when I first opened the PR, you must be right. I had definitely convinced myself that it fixed the issue but I was wrong.

> What you put in your Debian MR seems to be the same as in your PPA.
Yep, I was rushing and had incorrectly remembered that the patch had fixed the problem and was hoping it still would fix it.

> TBH I fear that there would have been a much simpler solution:
>
> https://gitlab.gnome.org/GNOME/glib/-/issues/2331#note_1067322
>
> But I saw that comment only when the fix was working, and now I'm disinclined to try it with the risk to find out that the work I did was wasted...

Darnit I should have caught this too.

@Gunnar, @Brian thanks so much for answering my questions. I'll try to learn from this experience and avoid these same mistakes next time :)