You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "lucasssvaz (via GitHub)" <gi...@apache.org> on 2023/05/26 12:22:25 UTC
[GitHub] [nuttx] lucasssvaz opened a new pull request, #9400: tools: Add way to get current defconfig name on runtime
lucasssvaz opened a new pull request, #9400:
URL: https://github.com/apache/nuttx/pull/9400
## Summary
This commit aims to provide a way to get the current applied defconfig on runtime by providing CONFIG_BASE_DEFCONFIG that is updated when the configure script is run.
## Impact
Slight modifications to the configure and build makefile scripts
## Testing
Tested on ESP32-DevKitC using a modified version of Nxdiag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212417720
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
@patacongo I tried to create the config as you suggested but the value is always the default. If I make it a named config, it works as expected. Any idea why ?
```
config BASE_DEFCONFIG
string
default ""
```
Generates:
```
#
# Build Configuration
#
CONFIG_APPS_DIR="../apps"
CONFIG_BASE_DEFCONFIG=""
CONFIG_BUILD_FLAT=y
# CONFIG_BUILD_2PASS is not set
```
But,
```
config BASE_DEFCONFIG
string "Base defconfig"
default ""
```
Generates
```
#
# Build Configuration
#
CONFIG_APPS_DIR="../apps"
CONFIG_BASE_DEFCONFIG="esp32-devkitc:nsh"
CONFIG_BUILD_FLAT=y
# CONFIG_BUILD_2PASS is not set
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212096648
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
> Then I'll fuse both ways that I've implemented this to add this info to `config.h` directly.
As @patacongo said that it's a normal practice to append some dynamical CONFIG to .config(you can see how CONFIG_APPS_DIR, CONFIG_HOST_XXX work).
> Doesn't it make sense to provide a `.info` containing not only the `CONFIG_BASE_DEFCONFIG` but also the diff between it and the current config? That would be very useful to check which configs were removed or added from a specific `defconfig`.
But "make savedefconfig" will update defconfig to reflect your change in .config.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212032639
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
For now .info contains only CONFIG_BASE_DEFCONFIG. I can't append it to config file as it isn't a Kconfig option (I will remove the CONFIG as it is misleading).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212042703
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
configure.sh already appends settings to .config. It is a standard practice to do this. You would need to have a definition in a Kconfig file, but only to prevent the appended setting from being lost if re-configured. Like:
config BASE_DEFCONFIG
string
default ""
It does not have to have a value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212161816
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
For C code, value of each configuration setting is available in include/nuttx/config.h
Shell scripts and Makfiles can just include .config to get the value of a setting.
That really couldn't be simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212088193
##########
tools/configure.sh:
##########
@@ -307,3 +309,12 @@ fi
# reconstitued before they can be used.
${TOPDIR}/tools/sethost.sh $host $*
+
+# Update the CONFIG_BASE_DEFCONFIG setting in the .info file
+
+posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
+echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" > "${info_file}"
+
+# Save the original configuration file for later comparison
Review Comment:
Yeah but it may be overwritten by a `make savedefconfig`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1565045179
Another thing that I would like to know is: Would you guys prefer to maintain only one file and merge the current `version.h` into the new `info.h` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1218135312
##########
tools/Win.mk:
##########
@@ -235,6 +235,15 @@ tools\mkconfig$(HOSTEXEEXT):
$(Q) $(MAKE) -C tools -f Makefile.host mkconfig$(HOSTEXEEXT)
include\nuttx\config.h: $(TOPDIR)\.config tools\mkconfig$(HOSTEXEEXT)
+ $(Q) grep -v "CONFIG_BASE_DEFCONFIG" "$(TOPDIR)\.config" > "$(TOPDIR)\.config.tmp"
+# In-place edit can mess up permissions on Windows
Review Comment:
To detect if the configuration was changed and appending "-dirty" if it was (just like the version file)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1565545180
> > Another thing that I would like to know is: Would you guys prefer to maintain only one file and merge the current `version.h` into the new `info.h` ?
>
> since it's only used by nxdiag, why not use a python script like https://github.com/apache/nuttx-apps/blob/master/tools/host_sysinfo.py to generate this information?
@xiaoxiang781216 since this is a simple script and it could become a de-facto solution for all other boards, I think it is better keep it in C instead of python. We don't want to depend on python for basic things, it could prevent us to compile NuttX inside a machine running NuttX in the future (case we get some compiler like tcc running on NuttX).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212035355
##########
tools/configure.sh:
##########
@@ -307,3 +309,12 @@ fi
# reconstitued before they can be used.
${TOPDIR}/tools/sethost.sh $host $*
+
+# Update the CONFIG_BASE_DEFCONFIG setting in the .info file
+
+posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
+echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" > "${info_file}"
+
+# Save the original configuration file for later comparison
Review Comment:
The .config.old contains a copy of the previous config applied and not the original one. If you do more than one change to the configuration it won't represent the original one anymore.
We will use this to detect if the configuration was changed or one of the defconfigs are being used. (similar to the version file)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212161816
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
For C code, the value of each configuration setting is available in include/nuttx/config.h
Shell scripts and Makfiles can just include .config to get the value of a setting.
That really couldn't be simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1218137201
##########
tools/configure.c:
##########
@@ -1590,6 +1596,37 @@ static void refresh(void)
}
}
+static void save_original_config(void)
+{
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c.config", g_topdir, g_delim);
+ char *source_config = strdup(g_buffer);
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c.config.orig", g_topdir, g_delim);
Review Comment:
Because I've tested it and the .config.old file is used differently by kconfig-frontends and kconfiglib. We cannot assume how it works.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis merged pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #9400:
URL: https://github.com/apache/nuttx/pull/9400
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1214692445
##########
tools/configure.c:
##########
@@ -1590,6 +1596,37 @@ static void refresh(void)
}
}
+static void save_original_config(void)
+{
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c.config", g_topdir, g_delim);
+ char *source_config = strdup(g_buffer);
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c.config.orig", g_topdir, g_delim);
Review Comment:
why need save config.orig file
##########
tools/Win.mk:
##########
@@ -235,6 +235,15 @@ tools\mkconfig$(HOSTEXEEXT):
$(Q) $(MAKE) -C tools -f Makefile.host mkconfig$(HOSTEXEEXT)
include\nuttx\config.h: $(TOPDIR)\.config tools\mkconfig$(HOSTEXEEXT)
+ $(Q) grep -v "CONFIG_BASE_DEFCONFIG" "$(TOPDIR)\.config" > "$(TOPDIR)\.config.tmp"
+# In-place edit can mess up permissions on Windows
Review Comment:
@lucasssvaz why need edit the .config?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1565598185
> > > Another thing that I would like to know is: Would you guys prefer to maintain only one file and merge the current `version.h` into the new `info.h` ?
> >
> >
> > since it's only used by nxdiag, why not use a python script like https://github.com/apache/nuttx-apps/blob/master/tools/host_sysinfo.py to generate this information?
>
> @xiaoxiang781216 since this is a simple script and it could become a de-facto solution for all other boards, I think it is better keep it in C instead of python. We don't want to depend on python for basic things, it could prevent us to compile NuttX inside a machine running NuttX in the future (case we get some compiler like tcc running on NuttX).
It's fine to develop the tool by C or Python. My major concern is that it isn't good to generate it unconditionaly since this information is only used by nxdiag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1572359763
> @patacongo <https://github.com/patacongo> I tried to create the config
> as you suggested but the value is always the default. If I make it a
> named config, it works as expected. Any idea why
> |config BASE_DEFCONFIG string default ""|
|No, I don't know exactly why.
|
|Are you just appending the||CONFIG_BASE_DEFCONFIG| setting to the end
of end of the .config file? In that case, there will be two values of
|CONFIG_BASE_DEFCONFIG in the file.|
|It would be better to use kconfig-tweak to set the new, unique value.
|
> |config BASE_DEFCONFIG |
> |string "Base defconfig" |
|Perhaps it is better to let the user manually change the BASE_DEFCONFIG
anyway. Some users might want to do that and I think they would expect
it to work that way.|
|Greg
|
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212161816
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
You don't have to parse anything.
For C code, the value of each configuration setting is available in include/nuttx/config.h
Shell scripts and Makfiles can just include .config to get the value of a setting.
That really couldn't be simpler.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212417720
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
@patacongo I tried to create the config as you suggested but the value is always the default. If I make it a named config, it works as expected. Any idea why ?
```
config BASE_DEFCONFIG
string
default ""
```
Generates:
```
#
# Build Configuration
#
CONFIG_APPS_DIR="../apps"
CONFIG_BASE_DEFCONFIG=""
CONFIG_BUILD_FLAT=y
# CONFIG_BUILD_2PASS is not set
```
But,
```
config BASE_DEFCONFIG
string "Base defconfig"
default ""
```
Generates
```
#
# Build Configuration
#
CONFIG_APPS_DIR="../apps"
CONFIG_BASE_DEFCONFIG="esp32-devkitc:nsh"
CONFIG_BUILD_FLAT=y
# CONFIG_BUILD_2PASS is not set
```
Both doing the following in the configure script
```
posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" >> "${dest_config}"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1572439747
> @patacongo <https://github.com/patacongo> I tried to create the config as you suggested but the value is always the default. If I make it a named config, it works as expected. Any idea why |config BASE_DEFCONFIG string default ""|
> |No, I don't know exactly why. | |Are you just appending the||CONFIG_BASE_DEFCONFIG| setting to the end of end of the .config file? In that case, there will be two values of |CONFIG_BASE_DEFCONFIG in the file.| |It would be better to use kconfig-tweak to set the new, unique value. |
> |config BASE_DEFCONFIG | |string "Base defconfig" |
> |Perhaps it is better to let the user manually change the BASE_DEFCONFIG anyway. Some users might want to do that and I think they would expect it to work that way.| |Greg |
I'm appending it right after `CONFIG_APPS_DIR`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] acassis commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1567223455
@lucasssvaz I never know why the CI breaks, but mostly the CI is correct, it is some "unseen" issue that we lost much time to figure out. I will open an issue to ask suggestion how to improve our CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1567083476
@xiaoxiang781216 @acassis @tmedicci I think this information might be useful for other apps too. I agree that generating a new header is overkill. I would suggest merging this new `info.h` and the `version.h` into a single file. I would suggest keeping the name `info.h` as it is intuitive. The only downside of this approach is that it would be a braking change for any application that includes `nuttx/version.h` as they would need to change it to `nuttx/info.h`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212056104
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
Then I'll fuse both ways that I've implemented this to add this info to `config.h` directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212016813
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
so .info just contain CONFIG_BASE_DEFCONFIG? why not append to .config directly?
##########
tools/configure.sh:
##########
@@ -307,3 +309,12 @@ fi
# reconstitued before they can be used.
${TOPDIR}/tools/sethost.sh $host $*
+
+# Update the CONFIG_BASE_DEFCONFIG setting in the .info file
+
+posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
+echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" > "${info_file}"
+
+# Save the original configuration file for later comparison
Review Comment:
why need save the original config file if we already have .config and .config.old?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1213202440
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
I've updated the MR with your suggestions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1564447543
I'll re-implement this in another way
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1567164128
The CI will break due to the newly added file to `.gitignore`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1565227221
> Another thing that I would like to know is: Would you guys prefer to maintain only one file and merge the current `version.h` into the new `info.h` ?
since it's only used by nxdiag, why not use a python script like https://github.com/apache/nuttx-apps/blob/master/tools/host_sysinfo.py to generate this information?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212084411
##########
tools/configure.sh:
##########
@@ -307,3 +309,12 @@ fi
# reconstitued before they can be used.
${TOPDIR}/tools/sethost.sh $host $*
+
+# Update the CONFIG_BASE_DEFCONFIG setting in the .info file
+
+posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
+echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" > "${info_file}"
+
+# Save the original configuration file for later comparison
Review Comment:
but, the original config already save in configure.sh:
https://github.com/apache/nuttx/blob/master/tools/configure.sh#L280-L281
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212084411
##########
tools/configure.sh:
##########
@@ -307,3 +309,12 @@ fi
# reconstitued before they can be used.
${TOPDIR}/tools/sethost.sh $host $*
+
+# Update the CONFIG_BASE_DEFCONFIG setting in the .info file
+
+posboardconfig=`echo "${boardconfig}" | sed -e 's/\\\\/\\//g'`
+echo "CONFIG_BASE_DEFCONFIG=\"$posboardconfig\"" > "${info_file}"
+
+# Save the original configuration file for later comparison
Review Comment:
but, the original config is already saved by configure.sh:
https://github.com/apache/nuttx/blob/master/tools/configure.sh#L280-L281
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] patacongo commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "patacongo (via GitHub)" <gi...@apache.org>.
patacongo commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212042703
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
configure.sh already appends settings to .config. It is a standard practice to do this. You would need to have a dummy definition in a Kconfig file only to prevent the appended setting from being lost if re-configured. Like:
config BASE_DEFCONFIG
string
default ""
It does not have to have a value. The user will not be prompted for a value.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] tmedicci commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212060031
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
Doesn't it make sense to provide a `.info` containing not only the `CONFIG_BASE_DEFCONFIG` but also the diff between it and the current config? That would be very useful to check which configs were removed or added from a specific `defconfig`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] tmedicci commented on a diff in pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "tmedicci (via GitHub)" <gi...@apache.org>.
tmedicci commented on code in PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#discussion_r1212128761
##########
tools/configure.c:
##########
@@ -1590,6 +1590,48 @@ static void refresh(void)
}
}
+static void generate_info()
+{
+ /* Append the configuration information to the .info file */
+
+ FILE *stream;
+ char *tmp_boarddir;
+
+ snprintf(g_buffer, BUFFER_SIZE, "%s%c%s", g_topdir, g_delim, ".info");
+ stream = fopen(g_buffer, "w");
+ if (!stream)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to open %s for write mode: %s\n",
+ g_buffer, strerror(errno));
+ exit(EXIT_FAILURE);
+ }
+
+ tmp_boarddir = double_appdir_backslashes(g_boarddir);
+ if (NULL == tmp_boarddir)
+ {
+ fprintf(stderr,
+ "ERROR: Failed to double boarddir backslashes\n");
+ exit(EXIT_FAILURE);
+ }
+
+ fprintf(stream, "CONFIG_BASE_DEFCONFIG=\"%s\"\n", tmp_boarddir);
Review Comment:
> But "make savedefconfig" will update defconfig to reflect your change in .config.
Yes, but the `.info` would be used to be parsed by `nxdiag`. Thus, it'd need to get it from an already "ready-to-be-parsed" file. The current `.config` may be already "patched" and `make savedefconfig` would provide the `defconfig`. But how would the `nxdiag` run such a command while being built?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1572521024
For some reason `fvp-armv8r/nsh` is failing on Unix.mk line 693. Locally there's only this warning:
```
warning: (ARCH_CHIP_FVP_R52 && ARCH_CHIP_FVP_R82) selects ARMV8R_HAVE_GICv3 which has unmet direct dependencies (ARCH_ARM && ARCH_ARMV8R)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] lucasssvaz commented on pull request #9400: tools: Add way to get current defconfig name on runtime
Posted by "lucasssvaz (via GitHub)" <gi...@apache.org>.
lucasssvaz commented on PR #9400:
URL: https://github.com/apache/nuttx/pull/9400#issuecomment-1565040857
Just updated to a more flexible way that can also be used by other PR in the future. I would just like to know if `Win.mk` can use commands like `cmp` and `sed`. I assumed that it can as Cygwin has support for both of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org