Allow 'small-build' and 'big-build' to be used as options. #2409

pan- · 2016-08-10T08:51:48Z

These two flags are already tested in tools/toolchains/gcc.py but can't
be used at the moment because they are not part of the argument list of
the option parameter.

pan- · 2016-08-10T09:07:59Z

@bridadan @c1728p9 Could you review this ?

0xc0170 · 2016-08-10T13:28:01Z

Jenkins failures are being investigated.

LGTM

c1728p9 · 2016-08-10T13:34:22Z

Looks good to me as well

bridadan · 2016-08-10T15:07:10Z

Seems fine to me. @theotherjimmy ?

theotherjimmy · 2016-08-10T15:45:31Z

@pan- nailed it. This is exactly how we should be enabling them.

jupe · 2016-08-10T15:58:05Z

what this small/big-build option actually does? should it be documented somewhere?

bridadan · 2016-08-10T16:29:42Z

@c1728p9 Something else just ocurred to me, the big-build should really be standard-build to match with the definition in hal/targets.json.

@jupe pretty sure this isn't documented anywhere, it should be though. This option determines which standard library is used. The small build uses the "nano" version of the standard library, which saves space but it isn't thread safe. So it enables single-threaded mode and things like that.

jupe · 2016-08-10T16:38:19Z

Then this definitely should be documented very well here (=in this PR)! Without proper documentation this will be forgotten hidden feature which "doesn't exists" !

pan- · 2016-08-11T07:46:05Z

@bridadan standard-build is a much better name. If we follow that route, it also has to be renamed in toolchains/gcc.py. I can do that.

It would also be interesting to have an option which allows fearless users to use the RTOS even if the standard library is unsafe.

0xc0170 · 2016-08-11T08:06:04Z

@pan- Can you add documentation to these options as part of this patch? At least for those 2 new additions here.

@bridadan standard-build is a much better name. If we follow that route, it also has to be renamed in toolchains/gcc.py. I can do that.

What about small-build -> non-standard-build 😄

pan- · 2016-08-11T08:29:27Z

@jupe, @0xc0170 It is already documented in mbed-cli, this PR just fix the options list:

See here: https://github.com/ARMmbed/mbed-cli#compiling-your-program

--options <OPTIONS> to select compile options. 
Examples: "debug-info": will generate debugging information; 
"small-build" will use microlib/nanolib, but limit RTOS to single thread; 
"save-asm": will save the asm generated by the compiler

I'm happy to add documentation if needed the question is where ? There is just no README or anything in tools/*.

0xc0170 · 2016-08-11T11:53:01Z

@bridadan standard-build is a much better name. If we follow that route, it also has to be renamed in toolchains/gcc.py. I can do that.

+1

bridadan · 2016-08-11T17:01:30Z

@pan- Makes sense to me 👍

@0xc0170 I would say that non-standard-build is actually less descriptive then small-build, though it is less "parallel" to standard-build.

0xc0170 · 2016-08-12T05:05:22Z

@pan- Please update to standard-build

pan- · 2016-08-12T08:51:53Z

@0xc0170 Done, please review.

0xc0170 · 2016-08-12T09:06:28Z

@bridadan @c1728p9 @sg- @theotherjimmy Happy with the current state of this? small-build vs standard-build ?

bridadan · 2016-08-12T14:17:43Z

@pan- Muchas gracias!

@0xc0170 LGTM, though it'd probably be good to double check with @c1728p9

sg- · 2016-08-12T22:33:59Z

not to bikeshed but "xxx-build" doesn't translate to the behavior "single thread only mode" or that the standard library is selected by this flag. Given that small libraries technically could become thread safe it seems that the flag should be --std-lib and --small-lib given that the single thread only mode could change given that a small library could become thread safe

pan- · 2016-08-15T09:20:04Z

@sg- I'm happy to apply any change and --std-lib or --small-lib are really good one. Nonetheless, I would add that iff we follow this route, it would also makes sense to apply it to the default_build options and replace it by default_lib.

Also do you want this as a regular flag or a possible value in the options (--std-lib or -o --std-lib) ?

Please advise on what should go in this PR and what shouldn't.

0xc0170 · 2016-08-15T14:47:36Z

not to bikeshed but "xxx-build" doesn't translate to the behavior "single thread only mode" or that the standard library is selected by this flag. Given that small libraries technically could become thread safe it seems that the flag should be --std-lib and --small-lib given that the single thread only mode could change given that a small library could become thread safe

Same should be applied for default_build in the targets.json? I think we discussed it to name more closely to its intention - std_lib or as @pan's proposal default_lib.

Also do you want this as a regular flag or a possible value in the options (--std-lib or -o --std-lib) ?

Option?

Let's unify it and document it

sg- · 2016-08-15T17:49:57Z

Great - Sounds like default-lib where target.default-lib = std and -o --xxx-lib FTW

theotherjimmy · 2016-08-16T19:04:46Z

-o --xxx-lib is not an option on the table, argparse will barf on that.

The options are:

-o xxx-lib
--xxx-lib

sg- · 2016-08-16T19:37:01Z

OK -o xxx-lib

pan- · 2016-08-17T12:19:38Z

@sg- @0xc0170 It is done, please review.

theotherjimmy · 2016-08-19T18:37:35Z

Rebase please, then +1

These two flags are already tested in tools/toolchains/gcc.py but can't be used at the moment because they are not part of the argument list of the option parameter.

With this change, the name is more descriptive and aligned with the targets definitions in hal/targets.json.

…to use. * rename 'small-build' into 'small-lib' * rename 'standard-build' into 'std-lib'

pan- · 2016-08-23T14:54:12Z

@theotherjimmy I've rebased this PR against the latest master.

theotherjimmy · 2016-08-23T14:55:54Z

tools/options.py

@@ -75,7 +75,9 @@ def get_default_options_parser(add_clean=True, add_options=True):
                                  'run Goanna static code analyzer")'),
                            type=argparse_lowercase_hyphen_type(['save-asm',
                                                                 'debug-info',
-                                                                 'analyze'],
+                                                                 'analyze',
+								 'small-lib',


Indentation is off. It would be nice if you added a space :) This is not a blocker.

damn, vim!
I change this immediately.

Thanks a bunch! I appreciate the commitment to good style!

theotherjimmy · 2016-08-23T14:58:28Z

👍

* rename "default_build" property into "default_lib" * rename "standard" value for "default_build" into "std".

pan- · 2016-08-23T15:06:58Z

@theotherjimmy I've fixed the indentation of options and squash it with the last commit, their is no need to add such commit in the history.

theotherjimmy · 2016-08-23T15:15:41Z

@pan- Agreed. Good call on the squash.

0xc0170 · 2016-08-23T15:15:45Z

tools/toolchains/gcc.py

@@ -273,13 +273,13 @@ def __init__(self, target, options=None, notify=None, macros=None, silent=False,
        GCC.__init__(self, target, options, notify, macros, silent, TOOLCHAIN_PATHS['GCC_ARM'], extra_verbose=extra_verbose)

        # Use latest gcc nanolib
-        if "big-build" in self.options:
+        if "std-lib" in self.options:


As this breaks a user space (if they use big-build) I would do it non-breaking way for now, or at least to print a warning "use this new option, this is not supported anymore". I would go with non-breaking way

They can't use "big-build" options.py does not allow it to be passed on the command line. If they were using it, we would have had an issue about that by now.

@theotherjimmy I recall this can be set up in the settings script file as BUILD_OPTIONS ?

thus there is a chance someone did it :-)

sg- · 2016-08-23T15:35:11Z

/morph test

mbed-bot · 2016-08-23T17:10:22Z

Result: FAILURE

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

/morph test

Output

mbed Build Number: 684

Test failed!

pan- · 2016-08-24T08:48:13Z

@sg- @0xc0170 There is only one failure in the whole suite and this seem totally unrelated to this PR (features-net-feature_ipv4-tests-mbedmicro-net-nist_internet_time_service: features-net-feature_ipv4-tests-mbedmicro-net-nist_internet_time_service with GCC_ARM on K64F)

…..48609ae 48609ae Merge branch 'release_internal' into release_external 62d8586 Ignore ns_monitor when receiving Ack (ARMmbed#2417) 3323f36 Add support for Ethernet RA dns configuration d8e7d40 Iotthd 4239 (ARMmbed#2414) b46f3c6 add empty function for ws_stack_info_get fc97980 Changed RADIUS shared secret length to 16-bit value f827ffc Added information API to Wi-SUN and border router 8f1f9d5 EDFE error handling update 51bf94e Fix adaptation interface unit tests (ARMmbed#2409) 0860b57 FHSS_WS: Fixed reading unicast remaining slots (ARMmbed#2408) 4d8c03b Border Router RADIUS client basic authentication functionality (ARMmbed#2406) fbfada9 Adaptation IF: Allocate fragmentation buffer only if needed (ARMmbed#2407) 66f1bff Added storing of PAN version to NVM on BR 89826ce Iotthd 4224 (ARMmbed#2403) 3fc1ae2 BR EUI-64 is now selected for 4WH using PMKID on 4WH Message 1 af8438e Timing tool traces (ARMmbed#2401) 7938795 Fixed new PD data request for check if EDFE exchange is active. 85ab8fd Extented Frame exchange support 86b1f27 Merge pull request ARMmbed#2399 from ARMmbed/IOTTHD-4220 fed69e0 Add missing test method to ws_empty_functions 6b58e26 Add EDFE mode to Socket API setsockopt 1283077 Test API to adjust 6LoWPAN fragmentation MTU size (ARMmbed#2398) e787874 Init MAC MTU size based on driver MTU size (ARMmbed#2397) bf8e89e Ignore neighbors using unsupported channel function (ARMmbed#2395) 1c263fd FHSS exclude channel usage from mask and Brazilian Domain support 841dcbe MAC: Configurable data whitening (ARMmbed#2393) 9a10d66 Fix global address detection (ARMmbed#2392) f27fe86 Corrected network name and PAN ID change on auth start bcce0ed Clarified border router routing table API description e4630a4 Wi-SUN interface now informs address changes as interface events 2174374 Fix error found by coverity (ARMmbed#2389) 843254a MPL: traces for transmit and receive message (ARMmbed#2387) git-subtree-dir: features/nanostack/sal-stack-nanostack git-subtree-split: 48609ae

…3fe574..48609ae 48609ae Merge branch 'release_internal' into release_external 62d8586 Ignore ns_monitor when receiving Ack (ARMmbed#2417) 3323f36 Add support for Ethernet RA dns configuration d8e7d40 Iotthd 4239 (ARMmbed#2414) b46f3c6 add empty function for ws_stack_info_get fc97980 Changed RADIUS shared secret length to 16-bit value f827ffc Added information API to Wi-SUN and border router 8f1f9d5 EDFE error handling update 51bf94e Fix adaptation interface unit tests (ARMmbed#2409) 0860b57 FHSS_WS: Fixed reading unicast remaining slots (ARMmbed#2408) 4d8c03b Border Router RADIUS client basic authentication functionality (ARMmbed#2406) fbfada9 Adaptation IF: Allocate fragmentation buffer only if needed (ARMmbed#2407) 66f1bff Added storing of PAN version to NVM on BR 89826ce Iotthd 4224 (ARMmbed#2403) 3fc1ae2 BR EUI-64 is now selected for 4WH using PMKID on 4WH Message 1 af8438e Timing tool traces (ARMmbed#2401) 7938795 Fixed new PD data request for check if EDFE exchange is active. 85ab8fd Extented Frame exchange support 86b1f27 Merge pull request ARMmbed#2399 from ARMmbed/IOTTHD-4220 fed69e0 Add missing test method to ws_empty_functions 6b58e26 Add EDFE mode to Socket API setsockopt 1283077 Test API to adjust 6LoWPAN fragmentation MTU size (ARMmbed#2398) e787874 Init MAC MTU size based on driver MTU size (ARMmbed#2397) bf8e89e Ignore neighbors using unsupported channel function (ARMmbed#2395) 1c263fd FHSS exclude channel usage from mask and Brazilian Domain support 841dcbe MAC: Configurable data whitening (ARMmbed#2393) 9a10d66 Fix global address detection (ARMmbed#2392) f27fe86 Corrected network name and PAN ID change on auth start bcce0ed Clarified border router routing table API description e4630a4 Wi-SUN interface now informs address changes as interface events 2174374 Fix error found by coverity (ARMmbed#2389) 843254a MPL: traces for transmit and receive message (ARMmbed#2387) git-subtree-dir: connectivity/nanostack/sal-stack-nanostack git-subtree-split: 48609ae

pan- mentioned this pull request Aug 17, 2016

Linker on Nordic nRF51822 cannot move location counter backwards. #2467

Closed

sg- added needs: work needs: CI labels Aug 23, 2016

pan- added 2 commits August 23, 2016 15:37

Allow 'small-build' and 'big-build' to be used as options.

cce10e2

These two flags are already tested in tools/toolchains/gcc.py but can't be used at the moment because they are not part of the argument list of the option parameter.

Rename big-build option into standard-build.

afde624

With this change, the name is more descriptive and aligned with the targets definitions in hal/targets.json.

Improve naming of the command line option which select the c library …

73a9258

…to use. * rename 'small-build' into 'small-lib' * rename 'standard-build' into 'std-lib'

pan- force-pushed the enable_small_and_big_build_options branch from 4acbe72 to 7760d6c Compare August 23, 2016 14:51

theotherjimmy reviewed Aug 23, 2016
View reviewed changes

Make selection of c library used consistent.

fe322ad

* rename "default_build" property into "default_lib" * rename "standard" value for "default_build" into "std".

pan- force-pushed the enable_small_and_big_build_options branch from 7760d6c to fe322ad Compare August 23, 2016 15:05

0xc0170 reviewed Aug 23, 2016
View reviewed changes

sg- added ready for merge and removed needs: CI needs: work labels Aug 24, 2016

sg- merged commit 5197edc into ARMmbed:master Aug 25, 2016

sg- added release-version: 5.1.3 and removed ready for merge labels Aug 25, 2016

screamerbg mentioned this pull request Aug 29, 2016

Revert "Allow 'small-build' and 'big-build' to be used as options." #2568

Closed

bridadan mentioned this pull request Sep 29, 2016

Release mbed-os-5.1.5 and mbed lib v127 #2850

Merged

pan- deleted the enable_small_and_big_build_options branch July 3, 2018 11:03

Allow 'small-build' and 'big-build' to be used as options. #2409

Allow 'small-build' and 'big-build' to be used as options. #2409

Conversation

pan- commented Aug 10, 2016

pan- commented Aug 10, 2016

0xc0170 commented Aug 10, 2016

c1728p9 commented Aug 10, 2016

bridadan commented Aug 10, 2016

theotherjimmy commented Aug 10, 2016

jupe commented Aug 10, 2016

bridadan commented Aug 10, 2016

jupe commented Aug 10, 2016 • edited Loading

pan- commented Aug 11, 2016

0xc0170 commented Aug 11, 2016

pan- commented Aug 11, 2016

0xc0170 commented Aug 11, 2016

bridadan commented Aug 11, 2016

0xc0170 commented Aug 12, 2016

pan- commented Aug 12, 2016

0xc0170 commented Aug 12, 2016 • edited Loading

bridadan commented Aug 12, 2016

sg- commented Aug 12, 2016

pan- commented Aug 15, 2016 • edited Loading

0xc0170 commented Aug 15, 2016 • edited Loading

sg- commented Aug 15, 2016

theotherjimmy commented Aug 16, 2016

sg- commented Aug 16, 2016

pan- commented Aug 17, 2016

theotherjimmy commented Aug 19, 2016

pan- commented Aug 23, 2016

theotherjimmy Aug 23, 2016 • edited Loading

Choose a reason for hiding this comment

pan- Aug 23, 2016

Choose a reason for hiding this comment

theotherjimmy Aug 23, 2016

Choose a reason for hiding this comment

theotherjimmy commented Aug 23, 2016

pan- commented Aug 23, 2016

theotherjimmy commented Aug 23, 2016

0xc0170 Aug 23, 2016

Choose a reason for hiding this comment

theotherjimmy Aug 23, 2016 • edited Loading

Choose a reason for hiding this comment

0xc0170 Aug 25, 2016 • edited Loading

Choose a reason for hiding this comment

sg- commented Aug 23, 2016

mbed-bot commented Aug 23, 2016

Result: FAILURE

Output

pan- commented Aug 24, 2016

jupe commented Aug 10, 2016 •

edited

Loading

0xc0170 commented Aug 12, 2016 •

edited

Loading

pan- commented Aug 15, 2016 •

edited

Loading

0xc0170 commented Aug 15, 2016 •

edited

Loading

theotherjimmy Aug 23, 2016 •

edited

Loading

theotherjimmy Aug 23, 2016 •

edited

Loading

0xc0170 Aug 25, 2016 •

edited

Loading