Re: [PSA] %meson contains -Db_ndebug=true

[Peter Hutterer]

On Mon, Apr 08, 2019 at 01:26:47PM +0200, Igor Gnatenko wrote:
> Ok, I have reverted change:
> 
> * https://bodhi.fedoraproject.org/updates/FEDORA-2019-42ea21c6e4
> * https://bodhi.fedoraproject.org/updates/FEDORA-2019-660ccd306c
> * https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-eb940f069c
> * https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-d229ead72a
> * https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-9f6eea54fe

thanks!

Cheers,
   Peter
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Tomasz Kłoczko]

On Tue, 9 Apr 2019 at 13:54, Igor Gnatenko 
wrote:

> At this moment,
>
> * %meson doesn't pass -Db_ndebug at all, we use project-specific options
> * If project doesn't specify it, b_ndebug=false is default in meson
> * In case of mesa, it specifies b_ndebug=if-release which works correctly
> now with backported patch
>

IMO discussed here issue is more related to Fedora macros.

As current definition of the %meson uses --buildtype=plain
instead --buildtype=release and that build type is fixed and does not
provide any other way to redefine build type than just just pass
another --buildtype= fiddling anything around plain build
profile straight in meson source code may introduce more harm than good
thing.

IMO whole rpm macro suite should proved something like %build_type macro
which should be used transparently by %configure, %cmake and %meson.
Effectively for now Fedora IMO needs only as %build_type something like
"release" and "debug".
Any other type of the build should have some strict definition of the
propose.

IMO it would be even good to have in whole rawhide development cycle
something like build all packages with build type "debug" and use "release"
type only after branching all packages and rebuild everything with build
type "release".
This would as well guarantee that whatever will be offered in new stable
release will be building with all other packages (which up to now still is
not the case).

kloczek
-- 
Tomasz Kłoczko | LinkedIn: http://lnkd.in/FXPWxH
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[mcatanzaro]

On Tue, Apr 9, 2019 at 7:54 AM, Kalev Lember  
wrote:

Awesome, thanks Igor! All of this sounds good to me.


Yeah, sounds like everything is fine now after the if-release fix. 
Thanks!


___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Kalev Lember]

Awesome, thanks Igor! All of this sounds good to me.

Kalev

On 4/9/19 14:47, Igor Gnatenko wrote:

At this moment,

* %meson doesn't pass -Db_ndebug at all, we use project-specific options
* If project doesn't specify it, b_ndebug=false is default in meson
* In case of mesa, it specifies b_ndebug=if-release which works 
correctly now with backported patch


On Mon, Apr 8, 2019 at 4:29 PM Kalev Lember > wrote:



On 4/8/19 15:16, mcatanz...@gnome.org 
wrote:
 > - Plain builds should match the behavior of release builds, and
Fedora
 > plain builds should match the behavior of upstream plain builds.
This is
 > what we briefly had but just reverted. (Option one)

I believe this is the case already in upstream meson git master. Igor,
correct me if I'm wrong.

-- 
Kalev

___
devel mailing list -- devel@lists.fedoraproject.org

To unsubscribe send an email to devel-le...@lists.fedoraproject.org

Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives:
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org


___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org


___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Igor Gnatenko]

At this moment,

* %meson doesn't pass -Db_ndebug at all, we use project-specific options
* If project doesn't specify it, b_ndebug=false is default in meson
* In case of mesa, it specifies b_ndebug=if-release which works correctly
now with backported patch

On Mon, Apr 8, 2019 at 4:29 PM Kalev Lember  wrote:

>
> On 4/8/19 15:16, mcatanz...@gnome.org wrote:
> > - Plain builds should match the behavior of release builds, and Fedora
> > plain builds should match the behavior of upstream plain builds. This is
> > what we briefly had but just reverted. (Option one)
>
> I believe this is the case already in upstream meson git master. Igor,
> correct me if I'm wrong.
>
> --
> Kalev
> ___
> devel mailing list -- devel@lists.fedoraproject.org
> To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
>
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[mcatanzaro]

On Mon, Apr 8, 2019 at 1:39 PM, Benjamin Tissoires 
 wrote:

That's not how I read it:
https://github.com/mesonbuild/meson/blob/master/docs/markdown/Builtin-options.md#base-options
-DB-ndebug defaults to false.


OK, then I was totally wrong, thanks.

Shame the docs are so hard to search through. :(


Again, this setting should be set per project, so reverting the change
was the right call IMO.


OK, I agree now. Since it's off by default in meson upstream, we 
shouldn't be turning it on in Fedora. Sounds like reverting is the 
right move.


With the above in mind, I consider mesa's meson build system to be 
broken for incorrectly assuming distros will use -Db_ndebug=if-release 
or -Db_ndebug=true, since we will not be doing that. So mesa should 
find another way to ensure its performance-sensitive asserts are not 
enabled in plain builds (or release builds). Right? We can of course 
work around the issue in the Fedora spec by using -Db_ndebug=true just 
for mesa.


Michael

___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Benjamin Tissoires]

On Mon, Apr 8, 2019 at 3:17 PM  wrote:
>
> On Mon, Apr 8, 2019 at 5:50 AM, Kalev Lember 
> wrote:
> > I agree as well. Please don't override -Db_ndebug in distro-wide
> > %meson
> > macro and instead move the override to mesa packaging if it's needed
> > there.
>
> Well hold up. -Db_ndebug defaults to if-release... right? We have only

That's not how I read it:
https://github.com/mesonbuild/meson/blob/master/docs/markdown/Builtin-options.md#base-options
-DB-ndebug defaults to false.

> changed the behavior for plain builds (distro builds) to match release
> builds? So any package that has ever tested a release build would
> already know to expect assertions to be disabled, and the only broken
> packages are those that have never tested a release build?

Packages built previously were tested against the default build system
and their test suite that catches the assert were passing. With the
-DB-ndebug changed to true, those same tests were failing.

>
> I see three options:
>
>  - Plain builds should match the behavior of release builds, and Fedora
> plain builds should match the behavior of upstream plain builds. This
> is what we briefly had but just reverted. (Option one)
>
>  - Upstream plain builds should be really completely plain. Fedora can
> either add on -Db_ndebug=true or not (options two and three). This
> makes less sense to me because -Db_ndebug is really special, not like
> other compiler flags.
>
> I don't think it's right to say "NDEBUG should be off by default
> because that's how it is with Autotools" since we're not Autotools
> anymore, we should decide something that makes sense for meson, based
> on meson semantics and meson expectations and in coordination with
> meson upstream. E.g. I see you (Igor) reverted the RPM macro change,
> but that was just the bandaid over the default behavior of if-release,
> which is actually still changed, right? It's unclear to me what the
> default value of -Db_ndebug is due to underdocumentation, but if it's
> if-release then this change is just going to reappear again in the next
> version of meson, right?

Again, this setting should be set per project, so reverting the change
was the right call IMO.

The default value is documented in the markdown file above and the
actual code is at
https://github.com/mesonbuild/meson/blob/master/mesonbuild/compilers/compilers.py#L399-L400
So definitively, defaults is `false` and the next meson release will
not change this as of today.

Cheers,
Benjamin
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Kalev Lember]

On 4/8/19 15:16, mcatanz...@gnome.org wrote:
- Plain builds should match the behavior of release builds, and Fedora 
plain builds should match the behavior of upstream plain builds. This is 
what we briefly had but just reverted. (Option one)


I believe this is the case already in upstream meson git master. Igor,
correct me if I'm wrong.

--
Kalev
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[mcatanzaro]

On Mon, Apr 8, 2019 at 5:50 AM, Kalev Lember  
wrote:
I agree as well. Please don't override -Db_ndebug in distro-wide 
%meson
macro and instead move the override to mesa packaging if it's needed 
there.


Well hold up. -Db_ndebug defaults to if-release... right? We have only 
changed the behavior for plain builds (distro builds) to match release 
builds? So any package that has ever tested a release build would 
already know to expect assertions to be disabled, and the only broken 
packages are those that have never tested a release build?


I see three options:

- Plain builds should match the behavior of release builds, and Fedora 
plain builds should match the behavior of upstream plain builds. This 
is what we briefly had but just reverted. (Option one)


- Upstream plain builds should be really completely plain. Fedora can 
either add on -Db_ndebug=true or not (options two and three). This 
makes less sense to me because -Db_ndebug is really special, not like 
other compiler flags.


I don't think it's right to say "NDEBUG should be off by default 
because that's how it is with Autotools" since we're not Autotools 
anymore, we should decide something that makes sense for meson, based 
on meson semantics and meson expectations and in coordination with 
meson upstream. E.g. I see you (Igor) reverted the RPM macro change, 
but that was just the bandaid over the default behavior of if-release, 
which is actually still changed, right? It's unclear to me what the 
default value of -Db_ndebug is due to underdocumentation, but if it's 
if-release then this change is just going to reappear again in the next 
version of meson, right?


Michael

___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Zbigniew Jędrzejewski-Szmek]

On Mon, Apr 08, 2019 at 12:50:06PM +0200, Kalev Lember wrote:
> 
> On 4/8/19 11:13, Florian Weimer wrote:
> >* Peter Hutterer:
> >
> >>On Mon, Apr 08, 2019 at 09:20:35AM +0200, Igor Gnatenko wrote:
> >>>Hello,
> >>>
> >>>While fixing mesa build which was slow due to a lot off debug stuff
> >>>(assert()), I've added -Db_ndebug=true into the %meson macro.
> >>>
> >>>This seemed like good approach because in Fedora we should not use debug
> >>>bits in runtime (not related to debuginfo). However this caused some
> >>>breakage in some packages like libratbag which was relying on assert() in
> >>>tests. This should be really fixed in affected packages..
> >>>
> >>>I am not convinced that we should revert meson change.. Let me know if you
> >>>think otherwise and why.
> >>
> >>IMO if asserts cause a slow build in mesa, mesa should disable asserts.
> >>
> >>Pushing this into the build system means you're changing the behaviour of
> >>every package build with meson. I rely on asserts in several packages for
> >>paths that must not happen - removing those asserts means instead of
> >>crashing we now have undefined behaviour, probably leaking things.
> >
> >I agree.
> >
> >Fedora should follow upstream defaults here, like most other
> >distributions.  Most upstreams leave assertions enabled.
> 
> I agree as well. Please don't override -Db_ndebug in distro-wide %meson
> macro and instead move the override to mesa packaging if it's needed there.

FWIW, we also build systemd in Fedora with assertions enabled in hope
of catching bugs early, and I was surprised when I noticed that it got
disabled by the macro. If the macro was being introduced now, I wouldn't
have a strong opinion whether b_ndebug=true or b_ndebug=false is the
right default. But I agree that we shouldn't *change* the setting, at
least not without making the change much more visible.

Zbyszek
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Igor Gnatenko]

Ok, I have reverted change:

* https://bodhi.fedoraproject.org/updates/FEDORA-2019-42ea21c6e4
* https://bodhi.fedoraproject.org/updates/FEDORA-2019-660ccd306c
* https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-eb940f069c
* https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-d229ead72a
* https://bodhi.fedoraproject.org/updates/FEDORA-MODULAR-2019-9f6eea54fe

On Mon, Apr 8, 2019 at 12:59 PM Kalev Lember  wrote:

>
> On 4/8/19 11:13, Florian Weimer wrote:
> > * Peter Hutterer:
> >
> >> On Mon, Apr 08, 2019 at 09:20:35AM +0200, Igor Gnatenko wrote:
> >>> Hello,
> >>>
> >>> While fixing mesa build which was slow due to a lot off debug stuff
> >>> (assert()), I've added -Db_ndebug=true into the %meson macro.
> >>>
> >>> This seemed like good approach because in Fedora we should not use
> debug
> >>> bits in runtime (not related to debuginfo). However this caused some
> >>> breakage in some packages like libratbag which was relying on assert()
> in
> >>> tests. This should be really fixed in affected packages..
> >>>
> >>> I am not convinced that we should revert meson change.. Let me know if
> you
> >>> think otherwise and why.
> >>
> >> IMO if asserts cause a slow build in mesa, mesa should disable asserts.
> >>
> >> Pushing this into the build system means you're changing the behaviour
> of
> >> every package build with meson. I rely on asserts in several packages
> for
> >> paths that must not happen - removing those asserts means instead of
> >> crashing we now have undefined behaviour, probably leaking things.
> >
> > I agree.
> >
> > Fedora should follow upstream defaults here, like most other
> > distributions.  Most upstreams leave assertions enabled.
>
> I agree as well. Please don't override -Db_ndebug in distro-wide %meson
> macro and instead move the override to mesa packaging if it's needed there.
>
> Kalev
> ___
> devel mailing list -- devel@lists.fedoraproject.org
> To unsubscribe send an email to devel-le...@lists.fedoraproject.org
> Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
> List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
> https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org
>
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Kalev Lember]

On 4/8/19 11:13, Florian Weimer wrote:

* Peter Hutterer:


On Mon, Apr 08, 2019 at 09:20:35AM +0200, Igor Gnatenko wrote:

Hello,

While fixing mesa build which was slow due to a lot off debug stuff
(assert()), I've added -Db_ndebug=true into the %meson macro.

This seemed like good approach because in Fedora we should not use debug
bits in runtime (not related to debuginfo). However this caused some
breakage in some packages like libratbag which was relying on assert() in
tests. This should be really fixed in affected packages..

I am not convinced that we should revert meson change.. Let me know if you
think otherwise and why.


IMO if asserts cause a slow build in mesa, mesa should disable asserts.

Pushing this into the build system means you're changing the behaviour of
every package build with meson. I rely on asserts in several packages for
paths that must not happen - removing those asserts means instead of
crashing we now have undefined behaviour, probably leaking things.


I agree.

Fedora should follow upstream defaults here, like most other
distributions.  Most upstreams leave assertions enabled.


I agree as well. Please don't override -Db_ndebug in distro-wide %meson
macro and instead move the override to mesa packaging if it's needed there.

Kalev
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Florian Weimer]

* Peter Hutterer:

> On Mon, Apr 08, 2019 at 09:20:35AM +0200, Igor Gnatenko wrote:
>> Hello,
>> 
>> While fixing mesa build which was slow due to a lot off debug stuff
>> (assert()), I've added -Db_ndebug=true into the %meson macro.
>> 
>> This seemed like good approach because in Fedora we should not use debug
>> bits in runtime (not related to debuginfo). However this caused some
>> breakage in some packages like libratbag which was relying on assert() in
>> tests. This should be really fixed in affected packages..
>> 
>> I am not convinced that we should revert meson change.. Let me know if you
>> think otherwise and why.
>
> IMO if asserts cause a slow build in mesa, mesa should disable asserts.
>
> Pushing this into the build system means you're changing the behaviour of
> every package build with meson. I rely on asserts in several packages for
> paths that must not happen - removing those asserts means instead of
> crashing we now have undefined behaviour, probably leaking things.

I agree.

Fedora should follow upstream defaults here, like most other
distributions.  Most upstreams leave assertions enabled.

Thanks,
Florian
___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org

Re: [PSA] %meson contains -Db_ndebug=true

[Peter Hutterer]

On Mon, Apr 08, 2019 at 09:20:35AM +0200, Igor Gnatenko wrote:
> Hello,
> 
> While fixing mesa build which was slow due to a lot off debug stuff
> (assert()), I've added -Db_ndebug=true into the %meson macro.
> 
> This seemed like good approach because in Fedora we should not use debug
> bits in runtime (not related to debuginfo). However this caused some
> breakage in some packages like libratbag which was relying on assert() in
> tests. This should be really fixed in affected packages..
> 
> I am not convinced that we should revert meson change.. Let me know if you
> think otherwise and why.

IMO if asserts cause a slow build in mesa, mesa should disable asserts.

Pushing this into the build system means you're changing the behaviour of
every package build with meson. I rely on asserts in several packages for
paths that must not happen - removing those asserts means instead of
crashing we now have undefined behaviour, probably leaking things.

NDEBUG is on the same level as _GNU_SOURCE, it significantly changes the
behaviour. Please leave it to packages to decide whether it should be set.

Cheers,
   Peter


___
devel mailing list -- devel@lists.fedoraproject.org
To unsubscribe send an email to devel-le...@lists.fedoraproject.org
Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: 
https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org