public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Wolfgang Denk <wd@denx.de>
To: Simon Glass <sjg@chromium.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
	Tom Rini <trini@konsulko.com>,
	Joe Hershberger <joe.hershberger@ni.com>
Subject: Re: [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file
Date: Wed, 06 Oct 2021 09:08:28 +0200	[thread overview]
Message-ID: <2772928.1633504108@gemini.denx.de> (raw)
In-Reply-To: <CAPnjgZ3WVvso8uSeKyiOg6oR_R=nkguGZYXGb9Ws-s5dmO5GmQ@mail.gmail.com>

Dear Simon,

In message <CAPnjgZ3WVvso8uSeKyiOg6oR_R=nkguGZYXGb9Ws-s5dmO5GmQ@mail.gmail.com> you wrote:
>
> > 1) This requires that the .env files are run through CPP, which is
> >    only added in a later patch.
>
> OK perhaps I should just merge the patches. It is a bit artificial
> having two and it seems that people agree we need the += syntax.

Yes, some way to append to the environment is useful, and using '+='
is an intuitive syntax for it.  I'm just not happy about sneeking in
a new, undocumented restriction on U-Boot variable names.  Yes,
there are probably not many systems around (if any) where a variable
name ends in a plus sign, but we should be consistent.

I have to admit that I don't have any nice alternative suggestion.
If we stick with the rule that only NUL and '=' cannot be used in a
variable name, we could write

	variable=+value

instead.  But this does not look as nice as '+=', and we need an
escape mechanism for the case where we want a simple assignment of a
value that starts with a '+'.


BTW: If we would add such a feature to U-Boot (which seems to be no
bad idea to me) we would probably implement an "env append" command?


> > 2) Even if I add an "#include board/<vendor>/env/common.env" in my
> >    <board>.env files, your logic would trigger on the existence of
> >    the common.env file and ignore the <board>.env files.
>
> OK, so I if reverse that, are you happy? What do you think about my
> explanation above?

The longer I think about it the more I wonder if any hard coded file
names are really a good way to handle this.  For example there might
be the case where we have 4 boards A, B, C and D, and boards A and B
would use one env file, and C and D would use another.  this does
not match the "<board>.env" scheme, and "common.env" does not fit
either, as we have two "common" files.

I wonder if there should rather be a Kconfig option so each board
can select it's env file name; default would be "<board>.env".
what do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Life would be so much easier if we could  just  look  at  the  source
code.                                                   -- Dave Olson

  parent reply	other threads:[~2021-10-06  7:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-02  0:38 [PATCH v5 0/5] env: Allow environment in text files Simon Glass
2021-10-02  0:38 ` [PATCH v5 1/5] sandbox: Drop distro_boot Simon Glass
2021-10-02  0:38 ` [PATCH v5 2/5] doc: Move environment documentation to rST Simon Glass
2021-10-04 12:05   ` Wolfgang Denk
2021-10-02  0:38 ` [PATCH v5 3/5] env: Allow U-Boot scripts to be placed in a .env file Simon Glass
2021-10-04  7:28   ` Rasmus Villemoes
2021-10-04 15:38     ` Tom Rini
2021-10-05 14:42     ` Simon Glass
2021-10-04 12:08   ` Wolfgang Denk
2021-10-05 14:42     ` Simon Glass
2021-10-05 14:55       ` Wolfgang Denk
2021-10-05 15:33         ` Simon Glass
2021-10-05 15:52           ` Tom Rini
2021-10-05 17:27             ` Simon Glass
2021-10-06  7:08           ` Wolfgang Denk [this message]
2021-10-02  0:38 ` [PATCH v5 4/5] env: Allow environment files to use the C preprocessor Simon Glass
2021-10-04 12:12   ` Wolfgang Denk
2021-10-02  0:38 ` [PATCH v5 5/5] sandbox: Use a text-based environment Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2772928.1633504108@gemini.denx.de \
    --to=wd@denx.de \
    --cc=joe.hershberger@ni.com \
    --cc=sjg@chromium.org \
    --cc=trini@konsulko.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox