Created attachment 202764 [details] add options Add DEBUG option
Created attachment 202941 [details] install-strip only if no debug
Hi, Thanks for the patch, I had a look. While I do agree with adding DEBUG support, I think it should be done the "proper" way. In the ports tree there is support for the "WITH_DEBUG" flag, which automatically disables optimizations, adds the -g flag and disables stripping of binaries(or at least tries to). If a port needs further flags they should be linked to this flag. I have also discovered that DEBUG options are not a good idea [1] So I'd rework this patch to not add a DEBUG option to the port but use the WITH_DEBUG (which is also triggere by WITH_DEBUG_PORTS) flag to check if extrra options should be enabled. I'd try to avoid a DEBUG option. Adding WITH_DEBUG_PORTS to make.conf is quite easy. [1] https://reviews.freebsd.org/D15773
(In reply to Guido Falsi from comment #2) Adding WITH_DEBUG_PORTS has less usability, IMHO, but ok. Review has no progress for a long time. :( Same changes can be done for devel/glib20.
(In reply to rozhuk.im from comment #3) > (In reply to Guido Falsi from comment #2) > > Adding WITH_DEBUG_PORTS has less usability, IMHO, but ok. Depends. If you want a bunch of ports with debugging symbols it's easier to use that than set DEBUG option in them. Anyway, even if providing an options we need to hook with WITH_DEBUG framework. > Review has no progress for a long time. :( The review I referenced is abandoned, since I got negative feedback and was convinced by the arguments there. > Same changes can be done for devel/glib20. Yes...well...I've been debugging both thunar and glib20 using "WITH_DEBUG" as is and did get useful backtraces... Anyway, will you be providing a revised patch? Otherwise I can cook it up, but not right away.
Created attachment 204173 [details] patch Now it check DEBUG option and WITH_DEBUG env var.
Why add DEBUG as an option? WITH_DEBUG is not enough to activate it, just adding --enable-debug as needed when WITH_DEBUG is on. While there is no strict rule about this and some ports are adding a DEBUG option, it is not a good practice, and using WITH_DEBUG should be the only way to enable it. it is not a user flag, it's a developer flag anyway.
(In reply to Guido Falsi from comment #6) --enable-debug does not affect to INSTALL_TARGET= install-strip, and as result binary without debug symbols, gdb bt show nothink usable. I see no other good ways to set INSTALL_TARGET=install, remove -O* and add -g to CFLAGS.
(In reply to rozhuk.im from comment #7) > (In reply to Guido Falsi from comment #6) > --enable-debug does not affect to INSTALL_TARGET= install-strip, and as > result binary without debug symbols, gdb bt show nothink usable. > I see no other good ways to set INSTALL_TARGET=install, remove -O* and add > -g to CFLAGS. Same reply as in bug 237714
Created attachment 204313 [details] Revised patch Revised patch attached. I simply removed the options knob, and leverage WITH_DEBUG only. I'm gong to commit this patch in the next days unless you have strong objections. Please understand I don't feel comfortable adding a DEBUG knob to port options, so I'll not do that. The rest of the patch is almost the same as what you proposed with only minor changes.
A commit references this bug: Author: madpilot Date: Fri May 10 17:45:55 UTC 2019 New revision: 501189 URL: https://svnweb.freebsd.org/changeset/ports/501189 Log: Enable extra debugging code when building binaries with debugging symbols. PR: 236438 Submitted by: rozhuk.im@gmail.com Changes: head/x11-fm/thunar/Makefile
Committed. Thanks!