You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/04/26 18:06:31 UTC
[GitHub] [incubator-nuttx] patacongo opened a new pull request #3611: tools/kconfig2html.c: Add missing parse logic
patacongo opened a new pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611
## Summary
I tried running tools/mkconfigars.sh for the first time in a long time and it failed due to some missing support in the parser: The following was supported
config FOO
int "Enable foo"
But not this equivalent form:
config FOO
int
prompt "Enable foo"
## Impact
Should have no impact since this is not a critical build tool.
NOTE: There are some other, unrelated issues:
$ tools/mkconfigvars.sh
File ./arch/risc-v/src/esp32c3/Kconfig Unhandled token: depends
Unrecognized garbage after default value
These are not currently addressed.
## Testing
Tested using tools/mkconfigvars.sh
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827928662
> Thanks. I didn't look at all of the of `depends on` in the file. When I new which one it was the fix was easy. We talked about this in the past and decided not to support condition comments of that form, that is why it is not support. The ones for the Expressif parts are inconsistent and the only cases where that form is used. Those comments probably should be removed (I know the Linux community encourages that usage, but it has been discourage in NuttX for many years so any usage is inconsistent).
No problem, I can submit a PR to remove the comments. I was used to working with Buildroot and I thought it could be interesting to add.
>
> BTW: The other
>
> ```
> depends on ARCH_CHIP_ESP32C3
> ```
>
> are unnecessary and redundant since the Kconfig file begins with:
>
> ```
> if ARCH_CHIP_ESP32C3
> ```
>
> Everything from there to the matching endif has the that dependency.
Thanks for the tip, I'll address that as well.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #3611: [DRAFT]tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827042644
This is marked a draft because significant additional effort will be required to get this through nxstyle, mostly because there are many long lines reported. This PR does at least provide a fix if this ever becomes an issue for anyone who is more motivated than I to deal with the nxstyle complaints.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827942328
> Thanks for the tip, I'll address that as well.
There are other issues as well. For example, people have begun using default values that are expressions of other configuration settings. The tool does not need to parse the expressions other than to know when they end.
But clearly this tools has not been used for many years and it would take more effort to get it back "up to snuff" and passing nxstyle issues. If the tools is truly unused, then should just be deleted rather than supported and fixed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827042644
This is marked a draft because additional effort will be required to get this through nxstyle, mostly because there are many long lines reported. This PR does at least provide a fix to the problem if this ever becomes an issue for anyone who is more motivated than I to deal with the nxstyle complaints.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827861678
> File ./arch/risc-v/src/esp32c3/Kconfig Unhandled token: depends
This error is related to this `comment` command:
```Kconfig
comment "Selected ESP32-C3 chip without embedded Flash, an external Flash memory is required."
depends on ARCH_CHIP_ESP32C3X
```
`arch/xtensa/src/esp32/Kconfig` contains a similar comment to let the user know why some options are hidden.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827923317
I have not looked into the other reported error, but here is a little more information from running within debug output enabled:
```
<li><a href="#CONFIG_ESP32C3_UART1_RXPIN">1.2.717.4 <code>CONFIG_ESP32C3_UART1_RXPIN</code>: UART1 RX Pin</a></li>
<li><a href="#CONFIG_ESP32C3_PARTITION">1.2.717.5 <code>CONFIG_ESP32C3_PARTITION</code>: ESP32-C3 Partition</a></li>
</ul>
Menu: Terminating token: config
<li><a name="menu_365_toc"><a href="#menu_365">1.2.718 Menu: Real-Time Timer</a></a></li>
<a href="#menu_365_toc" onclick="toggle('toggle_364', this)">Expand</a>
<ul id="toggle_364" style="display:none">
process_menu: TOKEN_MENU
kconfigdir: ./arch/risc-v/src/esp32c3
kconfigname: Kconfig
level: 4
CONFIG_ESP32C3_RT_TIMER_TASK_NAME: Terminating token: config
<li><a href="#CONFIG_ESP32C3_RT_TIMER_TASK_NAME">1.2.718.1 <code>CONFIG_ESP32C3_RT_TIMER_TASK_NAME</code>: Timer task name</a></li>
Unrecognized garbage after default value
```
It is probably complaining about this:
default 223 # Lower than high priority workqueue
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827920030
>
>
> > File ./arch/risc-v/src/esp32c3/Kconfig Unhandled token: depends
>
> This error is related to this `comment` command:
>
> ```
> comment "Selected ESP32-C3 chip without embedded Flash, an external Flash memory is required."
> depends on ARCH_CHIP_ESP32C3X
> ```
>
> `arch/xtensa/src/esp32/Kconfig` contains a similar comment to let the user know why some options are hidden.
Thanks. I didn't look at all of the of `depends on` in the file. When I new which one it was the fix was easy. We talked about this in the past and decided not to support condition comments of that form, that is why it is not support. The ones for the Expressif parts are inconsistent and the only cases where that form is used. Those comments probably should be removed (I know the Linux community encourages that usage, but it has been discourage in NuttX for many years so any usage is inconsistent).
BTW: The other
depends on ARCH_CHIP_ESP32C3
are unnecessary and redundant since the Kconfig file begins with:
if ARCH_CHIP_ESP32C3
Everything from there to the matching endif has the that dependency.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #3611: tools/kconfig2html.c: Add missing parse logic
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #3611:
URL: https://github.com/apache/incubator-nuttx/pull/3611#issuecomment-827942328
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org