search

sbabic / libubootenv

Generic library and tools to access and modify U-Boot environment from User Space
72 stars 39 forks source link

fw_printenv: dont hard-code configuration/environment files #22

Closed liuming50 closed 2 years ago

liuming50 commented 2 years ago

Avoid to hard-code configuration and environment files, allow them to be configurable during build. This is usefull when some end users want to distinguish these files at build time rather than run time with command parameters.

Signed-off-by: Ming Liu liu.ming50@gmail.com

liuming50 commented 2 years ago

Changes in patch set V2:

1 Also add a build option to CMAKE file, to make sure the default env and cfg can be passed by cmake command.

2 Fix some bad coding style in CMAKE file

sbabic commented 2 years ago

In the README file of the project is described how to contribute to the project. Patches should be sent to ML. Workflow is independent from the hoster (github, gitlab,etc) and uses other tools (patchwork) to collect patches and comments. By the way, the main question regarding this set is how a user (who is issueing fw_printenv / fw_setenv) can know where the configuration file are stored. The current behavior is equal to the one in old tools, where the path is hard-coded and fixed. In libubootenv, you can still change the behavior at runtime. But if a user is running fw_printenv without parameters (-c option), and there is no /etc/fw_env.config, how can he know where the file is located, maybe to fix it if this is just known at build time ?

liuming50 commented 2 years ago

In the README file of the project is described how to contribute to the project. Patches should be sent to ML. Workflow is independent from the hoster (github, gitlab,etc) and uses other tools (patchwork) to collect patches and comments. By the way, the main question regarding this set is how a user (who is issueing fw_printenv / fw_setenv) can know where the configuration file are stored. The current behavior is equal to the one in old tools, where the path is hard-coded and fixed. In libubootenv, you can still change the behavior at runtime. But if a user is running fw_printenv without parameters (-c option), and there is no /etc/fw_env.config, how can he know where the file is located, maybe to fix it if this is just known at build time ?

Sorry, will send this MR by mail list, I forgot the process.

Not sure if I understood you correctly, are you suggesting dropping -c/--defenv option? I attempt to say that options provide convenience for users to distinguish CFG/ENV files so maybe good to have. The reason I made this MR is just to provide user a way to choose default CFG/ENV files at build time, it should not introduce any functional changes at build/run time, the default DEFAULT CFG/ENV are not changed after applying this MR if the end users dont pass -DDEFAULT_CFG_FILE and -DDEFAULT_ENV_FILE on purpose.

sbabic commented 2 years ago

Not sure if I understood you correctly, are you suggesting dropping -c/--defenv option?

Absolutely not. libubootenv is mostly compatible with the u-boot-fw-tools as part of U-Boot project. The old tools have hardcoded paths: /etc/fw_env.config is fix. The library and the provided tools allow to change the path at run time. If a user is not happy with U-Boot's choice, it can set them at run time. If they are not set, the user knows that the path is coming from what U-Boot defined, that is /etc/fw_env.config.

Now you introduce to change them at build time - a user just runs fw_printenv / fw_setenv. How can he know which are the configuration files ? let's say you change that and you use /var/lib/dummy instead of /etc/fw_env.config. An user can be completely confused and he will try to change /etc/fw_env.config, because this is documented (in this project or in U-Boot). Different binaries running on different targets will have different configuration files. how can the user know what he should change if fw_printenv is not working ?

I attempt to say that options provide convenience for users to distinguish CFG/ENV files so maybe good to have. The reason I made this MR is just to provide user a way to choose default CFG/ENV files at build time, it should not introduce any functional changes at build/run time, the default DEFAULT CFG/ENV are not changed after applying this MR if the end users dont pass -DDEFAULT_CFG_FILE and -DDEFAULT_ENV_FILE on purpose.

This is clear. But there is no way to know which files are really used, and debugging this becomes a challenge without analyzing how the tool is built (and getting the CONFIG_ then). We need also a way to print the used configuration file for this.

liuming50 commented 2 years ago

@sbabic Arha, I got you now, I am totally on board, will send a V3 by mail list adding information about which config/env is being compiled in with fw_printenv/fw_setenv

liuming50 commented 2 years ago

@sbabic closing this MR, I have sent a MR to swupdate mail list according to your last comment.