<ambasta>
dnkl:Hey, so I analyzed the issue a little more, and it seems like build type release automatically sets -O3 .. this in turn leads the compiler to discard [[unused]] functions
<ambasta>
The same doesn't apply by default to plain build types
<ambasta>
So even without changing DNDEBUG, plain builds fine as long as --Doptimization is passed
<ambasta>
I believe the unittest definitions themselves should be wrapped in indefs to avoid pulling them in when building in non debug mode w/o optimizations
<ambasta>
Since [[unused]] isn't automatically discarded by the compiler
cbb has joined #foot
<cbb>
ambasta: unused static functions should be discarded regardless of -O3 or UNUSED
<cbb>
the UNUSED attribute is only there to tell the compiler not to warn about it
<ambasta>
Well, gcc runs into linker issues with fdm functions defined in unittests when I build w/o optimization
<cbb>
hmm, I guess they don't get discarded if there's no optimization passes at all then
<cbb>
but UNUSED is only to suppress warnings
<cbb>
ambasta: can you tell which UNITTEST blocks cause linker errors?
<cbb>
is it just a few of them?
<ambasta>
19
<ambasta>
Not sure, haven't gotten passed 19
<ambasta>
I can try to identify by commenting out each one of then individually
<ambasta>
But the issue arises on linking libpgo
<cbb>
nah, no need to do that atm I think
<cbb>
but if you could give me a file and line number for one of them it'd be useful
<ambasta>
Specifically, terminal.c:(.text+0xb845): undefined reference to `fdm_init'
<cbb>
ambasta: yeah I can reproduce with those build flags
<cbb>
it's just the one UNITTEST block in terminal.c that fails
<cbb>
if I wrap that with an indef the build completes
<ambasta>
Replacing __attribute__(__unused__) with [[ maybe_unused ]] as a prefix and c2x also seems to wrok
<cbb>
I guess we should do something about it
<cbb>
but it's worth noting that it should never really happen with a normal set of build flags
<cbb>
it doesn't make any sense to run PGO on a build without optimization
<cbb>
and it's that combination of flags that causes it
<cbb>
if I run meson with e.g. --builtype=plain but without -Db_pgo=generate, it doesn't happen
<ambasta>
Well, yes.. since the issue is in linking pgo
<ambasta>
I don't quite understand what's wrong w/ that specific linkage
<cbb>
PGO does a 2-stage build
<ambasta>
Right, generate profile, then use it to build it
<cbb>
the first gathers profile data and the second uses that data to optimize the build
<ambasta>
And the reason for weird flags is trying to package the build
<cbb>
I guess you're using --builtype=plain because the meson docs say it's for packaging
<ambasta>
Yep
<cbb>
but the idea behind that mode is you pass your own optimization flags along with it
<cbb>
so if you really want to do a PGO build
<cbb>
you should definitely also be using at least -O2
novakane has joined #foot
<ambasta>
Well yes, but setting CFLAGS is left to end users in this distro
<ambasta>
And users might not necessarily set -OX
<w0rm>
ambasta: FWIW, if there's a known minimal -O level required it's OK to override user setup
<ambasta>
w0rm: isn't -OX defined in make.conf CFLAGS?
<ambasta>
Unless ebuilds enforce it
<w0rm>
ambasta: i.e. if they enable pgo flags always override/append -O3 for example
<ambasta>
Hmm, okay
<cbb>
w0rm: ambasta: yeah I agree that seems like the best solution
<cbb>
we should probably still do the #ifdef fix anyway
<ambasta>
w0rm, cbb, yeah.. I'm patching it w/ ifdef in the packaging as well
<ambasta>
Just in case
<w0rm>
ambasta: yeah, just read through gentoo devmanual and quoting "... it is extremely important that certain user preferences are honoured as far as possible. A good example is CFLAGS, which must be respected (selective filtering is fine, but outright ignoring is not)."
<w0rm>
ambasta: glibc itself adds -O2 :-)
<w0rm>
ambasta: grep -R 'append-flags.*-O' /usr/portage to see bunch more examples
<w0rm>
so - I would not worry about it
<ambasta>
Yeah, but most are filed as bugs w/ upstream :P
<dnkl>
ambasta: w0rm: cbb: last time I checked gcc still produced badly optimized PGO builds when done with -O2. At least -O3 is required. This started happening somewhere around gcc-10
<dnkl>
-O2 will still /work/ though
cbb has quit [Ping timeout: 245 seconds]
cbb has joined #foot
novakane has quit [Quit: WeeChat 3.3]
novakane has joined #foot
ambasta has quit [Ping timeout: 260 seconds]
<cbb>
dnkl: I wonder if the fdm_init() and fdm_destroy() could just be omitted from the UNITTEST...?
<cbb>
it seems to get rid of the linker errors and doesn't seem to cause any other problems
<cbb>
since the PGO fdm_*() stubs just `return true` without doing anything
<dnkl>
cbb: yeah, looks like setting term->fdm = NULL has no ill effects - test still runs in debug builds
<dnkl>
Ah, no... It doesn't
<dnkl>
Forgot you had to actually run the binary...
cbb has quit [Ping timeout: 245 seconds]
cbb has joined #foot
<cbb>
dnkl: it seems like the UNITTEST block in terminal.c could be moved to an external test suite, if such a thing is ever added
<dnkl>
cbb: agreed. Terminal.c has outgrown itself in general...
<cbb>
dnkl: I guess the natural thing to do would be to use check.h, like fcft does?
<dnkl>
cbb: maybe. I like the idea of unit tests running every time you start the program, as opposed to manually having to run a test suite
<cbb>
dnkl: yeah I guess that's one advantage of embedded tests
<cbb>
I normally use UNITTEST for checking non-extern functions
<cbb>
and got into the habit of running `make check` all the time
<cbb>
for external tests
<cbb>
was just a thought anyway...I'll leave it to your judgement
<dnkl>
cbb: sure, I'm not really against having a "regular" test suite. It'd be good for packaging as well, as it would allow you to test release builds (and not to mention, not having to run the binary)
tsujp has joined #foot
avane has quit [Ping timeout: 265 seconds]
avane has joined #foot
st3r4g has joined #foot
cbb has quit [Ping timeout: 265 seconds]
cbb has joined #foot
tsujp has quit [Ping timeout: 256 seconds]
<novakane>
on fnott latest commits, shouldn't with only max-timeout set to 4 using like `notify-send "test" "test"` rescpect the timeout?
<novakane>
with default-timeout not set
<dnkl>
Hmm, yes, that was the idea... Worked when I tested, but not now...
<dnkl>
I guess there'll be a 1.1.2 release soon :/
<novakane>
yeah I have some script using notify-send and I didn't understood why the notif would stay