[WPE][GTK] Remove pkg-config use from FindLibBacktrace.cmake

[mcatanzaro]

0ff5cc8

[WPE][GTK] Remove pkg-config use from FindLibBacktrace.cmake
https://bugs.webkit.org/show_bug.cgi?id=265813

Reviewed by Adrian Perez de Castro.

libbacktrace upstream does not distribute a pkg-config file. Apparently
some downstreams are doing this, but I'm not sure who, because this
software is not packaged in major distros like Debian or Fedora. It's
not likely to be, because it doesn't have any releases. Let's not
attempt to use a downstream-only pkg-config file. If pkg-config support
can be upstream, that would be great.

* Source/cmake/FindLibBacktrace.cmake:

Canonical link: https://commits.webkit.org/272292@main

1abc212

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt		✅ 🛠 gtk
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 29d445a)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt		✅ 🛠 gtk
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

[aperezdc]

Some distributions are applying the patch from ianlancetaylor/libbacktrace#92 to add a .pc file to their libbacktrace packages. I would rather keep a few lines in a .cmake module file.

Also your commit log does not explain why using pkg-config is an issue. Now, whether we want ENABLE_LIBBACKTRACE=ON by default or not, that's a different discussion.

[mcatanzaro]

Some distributions are applying the patch from ianlancetaylor/libbacktrace#92 to add a .pc file to their libbacktrace packages. I would rather keep a few lines in a .cmake module file.

What if upstream eventually merges a pkg-config with a different name, say backtrace.pc? It reminds me of the era when distros would maintain custom soversions for shared libraries when upstreams didn't version their libraries properly, which turned out to be a big mistake.

Shame there is no response from upstream in that issue. If Ian was willing to add the pkg-config file, then we wouldn't need to debate this....

[mcatanzaro]

It looks like this broke a couple API tests. I will figure out how to disable or skip them.

[mcatanzaro]

So, I'm pretty sure we should not depend on a pkg-config file that does not exist upstream. Let's keep that.

Do you want me to split this into two pull requests, or is it OK to land all together like this?

[aperezdc]

So, I'm pretty sure we should not depend on a pkg-config file that does not exist upstream. Let's keep that.

Sure, we can re-add it later if/when upstream ships a .pc file.

Do you want me to split this into two pull requests, or is it OK to land all together like this?

I would leave the option as PUBLIC ON and then we don't even need to consider wheter two different patches are needed 😉

At the very, very least we want this to be PUBLIC ${ENABLE_DEVELOPER_MODE} because using libbacktrace gets us symbols in backtraces even with symbol visibility attributes in use (and with LTO, and a few other cases).

[mcatanzaro]

I'll edit this pull request tomorrow to only remove the pkg-config stuff and not change the visibility or default value of the build option. (That said, it was me who originally encouraged this to be enabled by default, because I didn't look closely enough into libbacktrace and didn't realize it doesn't have any releases.)

At the very, very least we want this to be PUBLIC ${ENABLE_DEVELOPER_MODE} because using libbacktrace gets us symbols in backtraces even with symbol visibility attributes in use (and with LTO, and a few other cases).

Honestly I have pretty low esteem for backtraces not generated by gdb, but I understand it's been difficult to add nicer core dump handling to the bots, and a libbacktrace backtrace is certainly a lot better than no backtrace. Still, this really seems primarily interesting for bots IMO. Developers should basically always be using gdb.

[webkit-early-warning-system]

EWS run on current version of this PR (hash 1abc212)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	❌ 🧪 ios-wk2-wpt		✅ 🛠 gtk
✅ 🛠 🧪 jsc	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc-arm64	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv-sim		✅ 🛠 jsc-armv7
✅ 🛠 🧪 merge	✅ 🛠 watch		✅ 🧪 jsc-armv7-tests
	✅ 🛠 watch-sim

[mcatanzaro]

(That said, it was me who originally encouraged this to be enabled by default, because I didn't look closely enough into libbacktrace and didn't realize it doesn't have any releases.)

To be clear: this package doesn't exist in Debian or Fedora, and lacking releases I have no interest in packaging it. So I expect all distros will turn this OFF. I regret suggesting that we should default it to ON. Oh well; we can always change that later, if desired.

[aperezdc]

I'll edit this pull request tomorrow to only remove the pkg-config stuff and not change the visibility or default value of the build option. (That said, it was me who originally encouraged this to be enabled by default, because I didn't look closely enough into libbacktrace and didn't realize it doesn't have any releases.)

Thanks, that sounds good.

At the very, very least we want this to be PUBLIC ${ENABLE_DEVELOPER_MODE} because using libbacktrace gets us symbols in backtraces even with symbol visibility attributes in use (and with LTO, and a few other cases).

Honestly I have pretty low esteem for backtraces not generated by gdb, but I understand it's been difficult to add nicer core dump handling to the bots, and a libbacktrace backtrace is certainly a lot better than no backtrace. Still, this really seems primarily interesting for bots IMO. Developers should basically always be using gdb.

OTOH, building in support for libbacktrace does help lots when using GDB to get a backtrace is not an option. Sometimes we don't have access to the hardware where issues arise, or it's underpowered to run full debug builds, and then having a backtrace with symbols and line numbers is much better than not having anything. It's also useful for release builds with symbols which get deployed to devices, although there we have a overlap with the Breakpad support.

[mcatanzaro]

Thanks, that sounds good.

Oh, I already did this. It's ready for review again.

[aperezdc]

r=me, thanks @mcatanzaro

[webkit-commit-queue]

Committed 272292@main (0ff5cc8): https://commits.webkit.org/272292@main

Reviewed commits have been landed. Closing PR #21294 and removing active labels.