[RFC PATCH] Provide mechanism for build-time default env entries

[Joel Johnson]

On January 20, 2020 5:15:37 PM UTC, Tom Rini <trini at konsulko.com> wrote:
>On Mon, Jan 20, 2020 at 10:09:21AM -0700, Joel Johnson wrote:
>> On 2020-01-20 09:51, Wolfgang Denk wrote:
>> > Dear Joel,
>> > 
>> > In message <20200119073738.29189-1-mrjoel at lixil.net> you wrote:
>> > > This enables the building user to specify environment values to
>be
>> > > included in the static default_environment with an image. This is
>> > > useful to build multiple otherwise like configured images,
>varying
>> > > by environment unique entries.
>> > > 
>> > > ---
>> > > 
>> > > I expected something like this to already be present, but
>couldn't
>> > > find such a mechanism. I also assumed that something similar may
>> > > have been proposed previously, but also couldn't find anything
>via
>> > > searching.
>> > ...
>> > 
>> > > +config USER_ENV_SETTINGS
>> > > +	string "User build-time additional environment entries"
>> > > +	help
>> > > +	  This value is reserved for the building user to provide
>custom
>> > > +	  environment entries to be added to the default environment.
>Care
>> > > must
>> > > +	  be taken to not break the environment, incompatible entries
>may
>> > > cause
>> > > +	  failure to compile, or failure of the target board to
>properly
>> > > +	  initialize or boot. The format is key=value pairs, with
>entries
>> > > +	  separated by C-style escaped null terminated values, such as:
>> > > +	    "key1=valueA\0key2=valueB\0key3=valueC".
>> > 
>> > Something in your descripotion must be missing, or this change
>makes
>> > no sense to me.
>> > 
>> > >  #ifdef	CONFIG_EXTRA_ENV_SETTINGS
>> > >  	CONFIG_EXTRA_ENV_SETTINGS
>> > > +#endif
>> > > +#ifdef	CONFIG_USER_ENV_SETTINGS
>> > > +	CONFIG_USER_ENV_SETTINGS
>> > >  #endif
>> > 
>> > What exactly is the difference between CONFIG_EXTRA_ENV_SETTINGS
>and
>> > CONFIG_USER_ENV_SETTINGS for you, i. e. what can you do with
>> > CONFIG_USER_ENV_SETTINGS that cannot be done in exatly the same way
>> > with CONFIG_EXTRA_ENV_SETTINGS?
>> > 
>> > Maybe it would help if you include any code that is actually using
>> > this feature?
>> > 
>> > Best regards,
>> > 
>> > Wolfgang Denk
>> 
>> The main different to me is that many boards set/replace
>> CONFIG_EXTRA_ENV_SETTINGS, making it unusable as a user build-time
>> configuration value. As a result, I can actually specify values for
>> CONFIG_USER_ENV_SETTINGS and have them included in the resulting
>environment
>> instead of overridden by the board configuration.
>> 
>> There is intentionally no code using this feature, as meant in the
>> description it is intended to be reserved for the building user's
>usage. The
>> specific use case for which I'm using it is to ease building of
>multiple
>> otherwise identical images, embedding a fixed and well known set of
>ethernet
>> MAC addresses into each unique image for execution in environments
>with no
>> environment storage. It is meant to be used for any other values
>changed or
>> added in the default image environment though - if there is a better
>way to
>> accomplish this I'd be all for it, but I couldn't find anything, thus
>the
>> RFC patch.
>
>I think part of the problem is that CONFIG_EXTRA_ENV_SETTINGS needs to
>be migrated to Kconfig.  And that in turn shows that some amount of
>things need to be taken out of the CONFIG namespace and moved in to
>distinct headers to be included / keyed-in-to.  What we have in
>include/environment/ti/ is a start to that but far from complete.

That sounds like a good path indeed, but wouldn't that still result in the Kconfig value being required to contain the full extra contents? The nice thing about a separate config entry is that it can serve as an overlay of just selected overrides and/or custom entries, instead of requiring following and updating user configs for in-source changes which are unrelated, but still captured in board level EXTRA

Either way works, I'd still be in favor of a separate overlay, thus the RFC patch. If you'd like it all eventually consolidated in EXTRA then I'll just carry the patch locally.

Joel