[3/4] docs: Add initial fw_setenv(1) manpage

Signed-off-by: Andreas Henriksson <andreas@fatal.se>
---
 docs/fw_setenv.1.scd | 95 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 95 insertions(+)
 create mode 100644 docs/fw_setenv.1.scd

Hi Andreas,

On 11.04.21 18:44, Andreas Henriksson wrote:
> Signed-off-by: Andreas Henriksson <andreas@fatal.se>
> ---
>   docs/fw_setenv.1.scd | 95 ++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 95 insertions(+)
>   create mode 100644 docs/fw_setenv.1.scd
> 
> diff --git a/docs/fw_setenv.1.scd b/docs/fw_setenv.1.scd
> new file mode 100644
> index 0000000..592652c
> --- /dev/null
> +++ b/docs/fw_setenv.1.scd
> @@ -0,0 +1,95 @@
> +fw_setenv(1) "April 2021" "libubootenv"
> +
> +# SYNOPSIS
> +
> +*fw_setenv* [option...] [[name] <value> | <script>]
> +
> +# DESCRIPTION
> +
> +This program provides a convenient way to access, parse and modify
> +the content of the U-Boot environment in a fully booted up system.
> +
> +It is similar to the *setenv* command available in the u-boot shell.
> +
> +# OPTIONS
> +
> +*-c, --config* <filename>
> +	configuration file (default: /etc/fw_env.config)
> +
> +*-f, --defenv* <filename>
> +	default environment if no one found
> +	(default: /etc/u-boot-initial-env)
> +
> +*-h, --help*
> +	print the help message and exit
> +
> +*-s, --script* <filename>
> +	read variables to be set from a script
> +
> +*-V, --version*
> +	print version and exit
> +
> +# EXAMPLES
> +
> +Set variable named `foobar` to value `this is a test`:
> +
> +```
> +fw_setenv foobar "this is a test"
> +```
> +
> +Delete a variable:
> +
> +```
> +fw_setenv foobar
> +```
> +
> +Set a variable to empty string:
> +
> +```
> +fw_setenv foobar ""
> +```
> +
> +Make sure default environment exists on disk:
> +
> +```
> +fw_setenv
> +```

Ok, this is a good trick. I will say fw_printenv without -f does the same.

> +
> +Create a dummy environment for testing:
> +
> +```
> +export TESTDIR="$(mktemp -d)"
> +cd "$TESTDIR"
> +echo "$TESTDIR/test.img 0x0 0x2000" >> fw_env.config
> +touch test.img
> +fw_setenv -c ./fw_env.config -f /dev/null

-f is optional, -f /dev/null makes no sense. So drop it.

> +fw_setenv -c ./fw_env.config foo bar
> +fw_setenv -c ./fw_env.config lala baba
> +fw_setenv -c ./fw_env.config sasa dada
> +fw_printenv -c ./fw_env.config
> +```
> +
> +# EXIT STATUS
> +
> +On success, 0 is returned, a non-zero failure code otherwise.
> +
> +# FILES
> +
> +*/etc/fw_env.config*
> +	The default configuration path. Describes where the u-boot
> +	environment lives. See *fw_env.config*(5) for detailed info.
> +
> +*/etc/u-boot-initial-env*
> +	The default u-boot environment to use if the location described
> +	in *fw_env.config* is uninitialized (or corrupted).
> +
> +# SEE ALSO
> +
> +*fw_env.config*(5), *fw_printenv*(1)
> +
> +# HISTORY
> +
> +The original implementation of the *fw_printenv* and *fw_setenv* tools lived
> +inside the u-boot repository. The version you're currently reading about is a
> +re-implementation of these tools, based on the *libubootenv* library.
> +
> 

Best regards,
Stefano Babic

Hello Stefano,

Thanks alot for your review feedback. A few comments below if you have
time to clarify it for me please.

I will look into rst2man as well as fixes for the other comments you
made when I find some spare time.

On Mon, Apr 12, 2021 at 11:25:26AM +0200, Stefano Babic wrote:
> Hi Andreas,
> 
> On 11.04.21 18:44, Andreas Henriksson wrote:
[...]
> > +Make sure default environment exists on disk:
> > +
> > +```
> > +fw_setenv
> > +```
> 
> Ok, this is a good trick. I will say fw_printenv without -f does the same.

Hmm... fw_printenv doesn't write to disk. I don't understand your
comment.

Note to self: 'default' should be replaced with 'an' to avoid confusion...
(Using fw_setenv does not *re*-set an existing environment to default values.)

> 
> > +
> > +Create a dummy environment for testing:
> > +
> > +```
> > +export TESTDIR="$(mktemp -d)"
> > +cd "$TESTDIR"
> > +echo "$TESTDIR/test.img 0x0 0x2000" >> fw_env.config
> > +touch test.img
> > +fw_setenv -c ./fw_env.config -f /dev/null
> 
> -f is optional, -f /dev/null makes no sense. So drop it.
[...]

I have to disagree with this review comment!
IMHO -f isn't very optional in practise in case your environment
does not yet exist on-disk.

Using -f /dev/null and not using -f at all is very different
(right now, maybe this is a bug?).

Without giving -f (and the environment is not on-disk, like in the
example where we create an empty test.img):

a/ If /etc/u-boot-initial-env exists, it's used .... and it's likely
  not empty so gives unexpected results (which might not matter
  here, but the example is inspired by a test-case I wrote).

b/ If /etc/u-boot-initial-env does not exist (and your environment is
  not on-disk!) you get an error!

Case b/ is likely the common case and something I hit myself while writing
a simple debian autopkgtest smoketest.
(And while on that, a test-suite might be useful to have to test all the
intricate details in the CLI people have come to rely on over the
years.)

(Maybe b/ should be changed to not error out on non-existant default
defenvfile?)


Regards,
Andreas Henriksson

PS. If anyone have other useful examples to add, please send them my
way!

Hi Andreas,

On 12.04.21 12:54, Andreas Henriksson wrote:
> Hello Stefano,
> 
> Thanks alot for your review feedback. A few comments below if you have
> time to clarify it for me please.
> 
> I will look into rst2man as well as fixes for the other comments you
> made when I find some spare time.
> 
> On Mon, Apr 12, 2021 at 11:25:26AM +0200, Stefano Babic wrote:
>> Hi Andreas,
>>
>> On 11.04.21 18:44, Andreas Henriksson wrote:
> [...]
>>> +Make sure default environment exists on disk:
>>> +
>>> +```
>>> +fw_setenv
>>> +```
>>
>> Ok, this is a good trick. I will say fw_printenv without -f does the same.
> 
> Hmm... fw_printenv doesn't write to disk. I don't understand your
> comment.
> 

"Make sure default environment exists on disk:"

If fw_printenv runs without -f, and a valid environment is printed (that 
means fw_printenv returns 0), there is a *valid* environment on disk.

If you run just fw_setenv, nothing happens. fw_setenv checks if there is 
a change, and if not, nothing is written. And to be sure that a default 
environment will be written, you must pass the -f option.

> Note to self: 'default' should be replaced with 'an' to avoid confusion...
> (Using fw_setenv does not *re*-set an existing environment to default values.)

There is no command (in user space) to restore the default environment. 
One way is near to this is:

	fw_setenv -f <default env> -f <default env>

(it does not delete additional vars outside the default env)

> 
>>
>>> +
>>> +Create a dummy environment for testing:
>>> +
>>> +```
>>> +export TESTDIR="$(mktemp -d)"
>>> +cd "$TESTDIR"
>>> +echo "$TESTDIR/test.img 0x0 0x2000" >> fw_env.config
>>> +touch test.img
>>> +fw_setenv -c ./fw_env.config -f /dev/null
>>
>> -f is optional, -f /dev/null makes no sense. So drop it.
> [...]
> 
> I have to disagree with this review comment!
> IMHO -f isn't very optional in practise in case your environment
> does not yet exist on-disk.

That is the reason. So yes, it should be always be passed if you know 
that the environment could be also not present on storage. But for the 
semantic of the command, it is optional: there is no error if -f is not 
passed.

> 
> Using -f /dev/null and not using -f at all is very different

-f /dev/null seems just to be buggy, it is not in the requested format, 
so what should be ?

> (right now, maybe this is a bug?).

I guess nobody had up now the idea to pass /dev/null. What is the 
intention ?

The default file should be a list of lines, each of them with
	varname=value

/dev/null ?

> 
> Without giving -f (and the environment is not on-disk, like in the
> example where we create an empty test.img):
> 
> a/ If /etc/u-boot-initial-env exists, it's used .... and it's likely
>    not empty so gives unexpected results (which might not matter
>    here, but the example is inspired by a test-case I wrote).

Unexpected results ? The reason for -f is to provide the same 
environment we have in U-Boot when there is no environment in storage. 
So it is to avoid unexpected results...

> 
> b/ If /etc/u-boot-initial-env does not exist (and your environment is
>    not on-disk!) you get an error!

And it is wanted...

> 
> Case b/ is likely the common case and something I hit myself while writing
> a simple debian autopkgtest smoketest.
> (And while on that, a test-suite might be useful to have to test all the
> intricate details in the CLI people have come to rely on over the
> years.)

Test units can be added, yes. But MTD cannot be easy used in test units, 
this drops a lot of test cases.

> 
> (Maybe b/ should be changed to not error out on non-existant default
> defenvfile?)

There is a lot of scripts just calling fw_setenv, and even if -f should 
be always passed, the tool must be compatible with the past. As the 
NDEBUG issue, you complain it is not like the old tool....

Best regards,
Stefano Babic

> 
> 
> Regards,
> Andreas Henriksson
> 
> PS. If anyone have other useful examples to add, please send them my
> way!
> 

Probably the most siugnificant part is fw_env.config. This is part of 
U-Boot's project, and there is some explanation (yes, documentation is 
not the best featur ein U-Boot):

# Configuration file for fw_(printenv/setenv) utility.
# Up to two entries are valid, in this case the redundant
# environment sector is assumed present.
# Notice, that the "Number of sectors" is not required on NOR and 
SPI-dataflash.
# Futhermore, if the Flash sector size is omitted, this value is assumed to
# be the same as the Environment size, which is valid for NOR and 
SPI-dataflash
# Device offset must be prefixed with 0x to be parsed as a hexadecimal 
value.

# NOR example
# MTD device name	Device offset	Env. size	Flash sector size	Number of 
sectors
/dev/mtd1		0x0000		0x4000		0x4000
/dev/mtd2		0x0000		0x4000		0x4000

# MTD SPI-dataflash example
# MTD device name	Device offset	Env. size	Flash sector size	Number of 
sectors
#/dev/mtd5		0x4200		0x4200
#/dev/mtd6		0x4200		0x4200

# NAND example
#/dev/mtd0		0x4000		0x4000		0x20000			2

# On a block device a negative offset is treated as a backwards offset 
from the
# end of the device/partition, rather than a forwards offset from the start.

# Block device example
#/dev/mmcblk0		0xc0000		0x20000
#/dev/mmcblk0		-0x20000	0x20000

# VFAT example
#/boot/uboot.env	0x0000          0x4000

# UBI volume
#/dev/ubi0_0		0x0		0x1f000		0x1f000
#/dev/ubi0_1		0x0		0x1f000		0x1f000

# UBI volume by name
#/dev/ubi0:env		0x0		0x1f000		0x1f000
#/dev/ubi0:env-redund	0x0		0x1f000		0x1f000


But it explains some more cases, and libubootenv is thought to be 
compatible with this format.

Best regards,
Stefano Babic

HI Andreas,

On Mon, Apr 12, 2021 at 1:32 PM Stefano Babic <sbabic@denx.de> wrote:
>
> Hi Andreas,
>
> On 12.04.21 12:54, Andreas Henriksson wrote:
> > Hello Stefano,
> >
> > Thanks alot for your review feedback. A few comments below if you have
> > time to clarify it for me please.
> >
> > I will look into rst2man as well as fixes for the other comments you
> > made when I find some spare time.
> >
> > On Mon, Apr 12, 2021 at 11:25:26AM +0200, Stefano Babic wrote:
> >> Hi Andreas,
> >>
> >> On 11.04.21 18:44, Andreas Henriksson wrote:
> > [...]
> >>> +Make sure default environment exists on disk:
> >>> +
> >>> +```
> >>> +fw_setenv
> >>> +```
> >>
> >> Ok, this is a good trick. I will say fw_printenv without -f does the same.
> >
> > Hmm... fw_printenv doesn't write to disk. I don't understand your
> > comment.
> >
>
> "Make sure default environment exists on disk:"
>
> If fw_printenv runs without -f, and a valid environment is printed (that
> means fw_printenv returns 0), there is a *valid* environment on disk.
>
> If you run just fw_setenv, nothing happens. fw_setenv checks if there is
> a change, and if not, nothing is written. And to be sure that a default
> environment will be written, you must pass the -f option.
>
> > Note to self: 'default' should be replaced with 'an' to avoid confusion...
> > (Using fw_setenv does not *re*-set an existing environment to default values.)
>
> There is no command (in user space) to restore the default environment.
> One way is near to this is:
>
>         fw_setenv -f <default env> -f <default env>
>
> (it does not delete additional vars outside the default env)

The behavior of -f to fill an uninitialized or broken environment is
in my personal experience a very useful feature. So I think it is
definitely worth documenting. See also
https://github.com/sbabic/libubootenv/commit/20d1ec784eda20f8b5a09e9dd8b186b1a3626b14

Cheers,
Mark