unix: Add build variants (analogous to boards on baremetal). #5253

jimmo · 2019-10-23T01:05:12Z

I don't have strong feelings on the name "variant", but "board" doesn't make sense. "target" is sort of the next best thing but also a bit overloaded.

Also, the "DEV" name for the default variant.. open to suggestions. Most people won't see this though? (Edit: updated to use "STANDARD", with "DEV" as the optional more-features build).

Some refactoring of mpconfigport.h might be worth doing now (especially common stuff between minimal and default).

Fixes #3043

(Edit: no longer enables help (ref #1354), will do that in a separate PR. I added settrace to the DEV variant as a different example).

ports/unix/variants/COVERAGE/mpconfigvariant.mk

dpgeorge · 2019-10-23T03:10:09Z

ports/unix/variants/DEV/mpconfigvariant.mk

@@ -0,0 +1,20 @@
+PROG ?= micropython


So the default make will now be the dev build, with executable named micropython... then the original base variant (got with make currently) is gone? Maybe we need a STANDARD variant (lean but featureful) and a DEV variant (all features, including settrace?)?

This was specifically to make it match the current behavior -- the default target produces the micropython binary.

Ok, but the default variant is now called "DEV", right? As below, maybe we need a BASE/STANDARD build and a DEV/FULL build.

dpgeorge · 2019-10-23T03:12:10Z

Also, the "DEV" name for the default variant.. open to suggestions. Most people won't see this though?

Yeah, that needs some discussion. Maybe it should be called "STANDARD" or "BASE". And then also a full development version could be the "DEV" variant, like discussed in #4309

ports/unix/Makefile

stinos · 2019-10-23T20:18:39Z

The name 'variant' sounds ok. Since 'DEV' leaves some room for interpretation, I'd rather go for more descriptive names which when looked at together should reveal more clearly what they might be, think minimal, standard, full and then eventually also have default as alias for standard?

Some general questions:

why capital names? Because it's the 'make' way perhaps? Also seems like with lowercase names some of those 'historical' targets would be fixed automatically but that's minor. And those capitalized directory names just look out of place :)
the mpconfigvariant.h file get included first now, so you already had to add some ifdefs to deal with that I think, couldn't it be included last so the main mpconfigport.h remains sort of clean and variants have to deal with it and can also do more like overriding existing definitions?
moving everything to separate directories seems pretty involved, but I assume this is the most flexible way in the end?

dpgeorge · 2019-10-24T05:43:25Z

Since 'DEV' leaves some room for interpretation, I'd rather go for more descriptive names which when looked at together should reveal more clearly what they might be, think minimal, standard, full and then eventually also have default as alias for standard?

Yes, minimal/standard/full is conceptually pretty good. Standard will be the current thing you get when you run "make", and full can be a build with everything enabled.

why capital names? Because it's the 'make' way perhaps? Also seems like with lowercase names some of those 'historical' targets would be fixed automatically but that's minor. And those capitalized directory names just look out of place :)

I guess it's following stm32/esp32/esp8266/samd upper-case naming (nrf uses lower case). Maybe they can be lower case because they are "variants" rather than "boards"? Or better to just be consistent with everything using upper-case?

the mpconfigvariant.h file get included first now, so you already had to add some ifdefs to deal with that I think, couldn't it be included last so the main mpconfigport.h remains sort of clean and variants have to deal with it and can also do more like overriding existing definitions?

stm32 includes mpconfigboard.h first in mpconfigport.h, which allows a board to enable things like threading, networking or bluetooth, then giving mpconfigport.h a chance to enable settings based on these board-level settings. Eg threading requires a different MICROPY_EVENT_POLL_HOOK and networking/bluetooth different built in modules and root pointers in MICROPY_PORT_ROOT_POINTERS. It's not possible to do this (easily) if mpconfigboard.h is included last.

Also, including it first matches how py/mpconfig.h includes mpconfigport.h first.

To reduce the need for ifdefs in mpconfigport.h, it's possible to move more of the config in mpconfigport.h to mpconfigvariant.h, and in mpconfigport.h only put those things that are enabled always for all variants. Downside of this is that common things that are enabled for most variants will be duplicated in all those variants.

So, probably the way it is is the "simplest", by some definition of simple :)

moving everything to separate directories seems pretty involved, but I assume this is the most flexible way in the end?

I assume you mean the subdirs for each variant in ports/unix/variants/? I think having separate subdirs for each variant is the only way it'll work, so they can all have the same name for mpconfigvariant.h, and the include dir just selects which one is included by mpconfigport.h.

jimmo · 2019-10-24T07:58:12Z

Also, the "DEV" name for the default variant.. open to suggestions. Most people won't see this though?

Yeah, that needs some discussion. Maybe it should be called "STANDARD" or "BASE". And then also a full development version could be the "DEV" variant, like discussed in #4309

Yeah, DEV is the wrong name for what this currently is. STANDARD seems fine as the default variant (identical to the current default target, producing the micropython binary, perhaps with a couple of small improvements like help / setattr, etc), and I will make DEV instead be the "one with all the features".

But to facilitate this I will add a BASE variant, which probably isn't one you'd build directly, but includes shared config for several of the variants (STANDARD, DEV, COVERAGE, etc). Also BASE can be the one that adds the test target, so it's available for these variants.

Since 'DEV' leaves some room for interpretation, I'd rather go for more descriptive names ... minimal, standard, full

Agreed.

Some general questions:

why capital names? Because it's the 'make' way perhaps? Also seems like with lowercase names some of those 'historical' targets would be fixed automatically but that's minor. And those capitalized directory names just look out of place :)
the mpconfigvariant.h file get included first now, so you already had to add some ifdefs to deal with that I think, couldn't it be included last so the main mpconfigport.h remains sort of clean and variants have to deal with it and can also do more like overriding existing definitions?
moving everything to separate directories seems pretty involved, but I assume this is the most flexible way in the end?

The simple, but maybe not good, answer to all of the above is "because that's how the STM port does it" (and by extension, NRF, SAMD, ESP32 and ESP8266) (although NRF is an exception to the capitalised names).

I'm keen to keep the configurations as similar as possible between ports, including things the order that the files get included in. The convention (as I understand it?) is that it's a deliberate choice to add the #ifndef to mpconfigport.h to "allow" a board to override something.

However, I think maybe adding the BASE config could help here -- so the order would be mpconfigvariant.h, which could then include BASE/mpconfigvariant.h at any point it likes, followed by mpconfigport.h.

stinos · 2019-10-24T09:15:49Z

Ok people, thanks for the extra explanation!

jimmo · 2019-10-28T02:41:01Z

Updated with feedback. STANDARD is now the default variant. I didn't do the refactoring via a "BASE" variant, that can come later.

dpgeorge · 2019-10-28T23:48:31Z

ports/unix/variants/FREEDOS/mpconfigvariant.mk

+MICROPY_PY_BTREE = 0
+
+MICROPY_PY_THREAD = 0
+


Probably don't need these empty lines between MICROPY_PY's, similar to bare-metal ports.

Done (and in MINIMAL too)

dpgeorge · 2019-10-28T23:52:31Z

py/mphal.h

@@ -34,6 +34,9 @@
 #include <mphalport.h>
 #endif

+// For uintptr_t
+#include <stdint.h>
+


This shouldn't be needed, I have fixed it in a different way in #5227 (define mp_hal_stdio_poll to nothing).

OK, grabbed the same fix from that PR.

This allows ports/variants to configure MICROPY_FORCE_32BIT after including mkenv.mk, but before py.mk.

This will eventually become the "full featured" unix binary with more features enabled, specifically useful for development and testing.

dpgeorge · 2020-01-12T00:07:15Z

Merged, see #5519

Fix GPIOTE crashes by checking everything is ok

dpgeorge reviewed Oct 23, 2019

View reviewed changes

dpgeorge added the port-unix label Oct 23, 2019

stinos reviewed Oct 23, 2019

View reviewed changes

ports/unix/Makefile Outdated Show resolved Hide resolved

jimmo force-pushed the unix-variants branch 2 times, most recently from 42d62f5 to 83b6de2 Compare October 28, 2019 02:38

dpgeorge reviewed Oct 28, 2019

View reviewed changes

jimmo force-pushed the unix-variants branch 2 times, most recently from 0aa9303 to 90bdbc1 Compare October 29, 2019 23:15

jimmo added 5 commits January 8, 2020 14:47

py/mkenv.mk: Move usage of 32-bit flags to py.mk.

dc36019

This allows ports/variants to configure MICROPY_FORCE_32BIT after including mkenv.mk, but before py.mk.

unix: Add build variants (analogous to boards on baremetal).

601b8f7

unix: Add placeholder DEV variant with settrace enabled.

434c903

This will eventually become the "full featured" unix binary with more features enabled, specifically useful for development and testing.

travis: Update travis to specify which variant to build.

21fe121

unix: Rename unix binaries to micropython-variant (not _variant).

32225f3

jimmo force-pushed the unix-variants branch from ccd7b18 to 32225f3 Compare January 8, 2020 04:04

dpgeorge mentioned this pull request Jan 11, 2020

unix: add build variants (v2) #5519

Merged

dpgeorge closed this Jan 12, 2020

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Aug 31, 2021

Merge pull request micropython#5253 from tannewt/fix_nrf_gpiote_crash

1dbeed8

Fix GPIOTE crashes by checking everything is ok

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

unix: Add build variants (analogous to boards on baremetal). #5253

unix: Add build variants (analogous to boards on baremetal). #5253

jimmo commented Oct 23, 2019 •

edited

dpgeorge Oct 23, 2019

jimmo Oct 24, 2019

dpgeorge Oct 24, 2019

dpgeorge commented Oct 23, 2019

stinos commented Oct 23, 2019

dpgeorge commented Oct 24, 2019

jimmo commented Oct 24, 2019

stinos commented Oct 24, 2019

jimmo commented Oct 28, 2019

dpgeorge Oct 28, 2019

jimmo Oct 29, 2019

dpgeorge Oct 28, 2019

jimmo Oct 29, 2019

dpgeorge commented Jan 12, 2020

		MICROPY_PY_BTREE = 0

		MICROPY_PY_THREAD = 0

unix: Add build variants (analogous to boards on baremetal). #5253

unix: Add build variants (analogous to boards on baremetal). #5253

Conversation

jimmo commented Oct 23, 2019 • edited

dpgeorge Oct 23, 2019

Choose a reason for hiding this comment

jimmo Oct 24, 2019

Choose a reason for hiding this comment

dpgeorge Oct 24, 2019

Choose a reason for hiding this comment

dpgeorge commented Oct 23, 2019

stinos commented Oct 23, 2019

dpgeorge commented Oct 24, 2019

jimmo commented Oct 24, 2019

stinos commented Oct 24, 2019

jimmo commented Oct 28, 2019

dpgeorge Oct 28, 2019

Choose a reason for hiding this comment

jimmo Oct 29, 2019

Choose a reason for hiding this comment

dpgeorge Oct 28, 2019

Choose a reason for hiding this comment

jimmo Oct 29, 2019

Choose a reason for hiding this comment

dpgeorge commented Jan 12, 2020

jimmo commented Oct 23, 2019 •

edited