[Proposal] Simplify build profiles: debug and release only

[bulislaw]

Description

I think our build profile story is a bit complicated and not clear. What's default profile? It doesn't mean much, requires checking documentation and leads to assumptions and issues. I think default build should be make in debug mode and as most environments/projects we should have clear release mode.

I've decided to remove default profile, make debug the default and rename small to release.

Let me know what do you think.

CC: @bridadan @theotherjimmy @0xc0170

[theotherjimmy]

@AnotherButler More build_profile.md modifications here.

[theotherjimmy]

I like the way this is done, and I have no problem with removing the default profile. I agree that default has no meaning by itself.

[geky]

+1 from me. Although this might push some examples over device limits in default build (now debug).

[bulislaw]

I have some more changes to build_profile.md in my pipeline (when #3566 hits master) and I'm aware about the external repo, so will merge it and submit PR agains it when it's all finished.

[bulislaw]

I'm assuming we are talking about nrf51? :) I'll test some of them on Monday.

[geky]

More just a warning that this might be a very breaking change. Should probably be on a minor release.

[AnotherButler]

I don't think y'all are working on the latest version of this document. It's here in the Handbook, here on GitHub.

[c1728p9]

We probably want to run this by @sg- before merging if that hasn't been done already. Building as debug in the online compiler is going to make the code bigger and slower than it was before and without much benefit since it can't be used with a debugger.

[bridadan]

I think this helps simplifies things quite a bit, thanks for bringing this up @bulislaw. Though like @geky said this may have some unintended consequence. We should probably run this by the Client team. I'll go ahead and mark this as do not merge for now until we've had time to investigate all the implications of this.

Looks like the Oulu CI found that it blows up the size of one of the mbed OS test apps already: https://jenkins-internal.mbed.com/job/ARMmbed/job/mbed-os-cliapp/job/master/2309/

@c1728p9 That's a really good point. How about we instead name the default profile to online or something. Then the online tools can be configured to use that profile instead? Then the default profile can be debug for offline tools.

[theotherjimmy]

@bridadan That suggestion (online and debug) seems like a poor user experience. A user would be confused when something works by default online but does not fit in the device offline.

[bulislaw]

Right, seems that beetle can't handle cordio built with debug info.

[sg-]

I dont understand what is confusing. There is a default profile which equals backwards compatible profile. We have a debug profile and then there is a near unless small profile which you may want to use for release if memory constrained. What am I missing here?

What does the default profile not do that y'all would like it to do?

[pan-]

More just a warning that this might be a very breaking change. Should probably be on a minor release.

It might be a breaking change for big examples, code without optimization will definitely consume more flash. It might also be a breaking change for users with big applications and if ever they just use the online IDE then such change makes mbed and its ecosystem non usable anymore because they won't have the possibility to switch to the release profile.

Beside breaking changes, I wonder if increasing the size of binary generated by default is the right message to send. Here is the flash consumed by blinky when compiled by GCC on the K64F:

small	default	debug
17916 bytes	47159 bytes	57399 bytes

Increasing the size of such simple application to almost 60K is terrifying for someone without the background of what is generated.

[bridadan]

Thanks for posting actual numbers, I didn't realize the impact would be that large

[ohagendorf]

aber klar doch, siehe Anhang Die ganze CAD Daten liegen auf der NAS. Es gibt dort eine CAD Freigabe ... Am 03.02.2017 um 22:17 schrieb Brian Daniels:

…

Thanks for posting actual numbers, I didn't realize the impact would be that large — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3693 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AINcSlG0CGOqk0UAsenvMjP6_Sdq6JvHks5rY5lwgaJpZM4L2hRr>.

-- Dr. Olaf Hagendorf RG Automation and Mechatronics (ATM) in the Computational Engineering and Automation Group (CEA) Hochschule Wismar - University of Applied Sciences: Technology, Business and Design Faculty of Engineering Department of electrical engineering and computer sience 23966 Wismar, Philipp-Müller-Straße 14, Germany Tel.: +49 3841 753 7176 Email: [email protected] www-oh: https://atm.eui.fiw.hs-wismar.de/dr-olaf-hagendorf.html www-cea: www.mb.hs-wismar.de/cea

[tommikas]

Looks like the Oulu CI found that it blows up the size of one of the mbed OS test apps already

Yeah, some configurations don't build for all devices with debug enabled.

Switching CI to use a different profile isn't a problem of course, but as has been pointed out it's a breaking change for now. If you want to go forward with this, I can fix continuous-integration/jenkins/pr-head so it works with this PR and doesn't break on merge to master.

[bulislaw]

@pan- I would say having that simple application taking up almost 50k, not in obvious 'debug mode' is terrifying in the first place. I mean we know exactly why it's so big, but still seeing it from the user's point of view is bad, especially when they get it by default.

@sg- I think most of people are used to seeing debug/release, which is not only de facto standard, but also it's self explanatory. I know what to use without thinking about it and I know what to expect from the build. In our current situation with 'default' I don't really know what will i get without checking. Also I was thinking about features, that may be enabled/disabled basing on used profile and i actually got it wrong (and I'm sure 99% of people will do some sort assumption rather than check) I was assuming we use default for 'everyday' development, debug when something goes wrong and they actually need to debug, and people would use small for deployment. So it's not about default profile not doing something i need, it's about making it more verbose and predictable.

That being said, I wasn't expect such a fallout from boards/testing, but what do people think about at least renaming default profile? We treat it as release as it seems, so maybe we can call it 'release'.

[0xc0170]

That being said, I wasn't expect such a fallout from boards/testing, but what do people think about at least renaming default profile? We treat it as release as it seems, so maybe we can call it 'release'.

It was always release for me, fine with me.

[pan-]

but what do people think about at least renaming default profile? We treat it as release as it seems, so maybe we can call it 'release'.

Given that a behavior like sleeping now depends on NDEBUG definition (see pr #3607) and the default profile doesn't define it because assertions have to be enabled; I believe default and small profiles are two different beasts with two very different behavior.

I was assuming we use default for 'everyday' development, debug when something goes wrong and they actually need to debug, and people would use small for deployment. So it's not about default profile not doing something i need, it's about making it more verbose and predictable.

I think you nailed it with a good terminology. There are three profiles: debug, development and release. Maybe renaming default into development and small into release would clear things up.

[bulislaw]

I agree debug, development and release are descriptive and sort of convey how they are intended to be used.

Also while we talk about the sleep, in this profile terminology, I would change it to work as a real sleep for both development and release (by disconnecting it from NDEBUG and introducing another flag, something like 'REAL_SLEEP' which could be defined for these profiles.

[bulislaw]

Updated to reflect the conversation we had.

[bulislaw]

/morph test

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1518

All builds and test passed!

[bridadan]

@jupe the "small" profile has been renamed to "release". Will that affect any of your CIs?

[tommikas]

@bridadan (Not Jussi, but...) I don't think so. We're currently using the default profile in CI builds. The name of the default changed but not the profile itself --> no effect.

[bridadan]

Great thanks @tommikas (sorry should have tagged you 😄)

[tommikas]

No worries. Jussi definitely could've answered the question too but he's currently on leave.