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 2020/04/27 17:33:15 UTC

[GitHub] [incubator-nuttx] patacongo opened a new pull request #897: tools/sethost.sh: Correct error in setting a different host.

patacongo opened a new pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897


   ## Summary
   
   In an older PR, the standard kconfig-tweak calls were replaced with sed edit.  This is an incorrect change and results in invalid configurations.  This change restores the use of kconfig-tweak and always generates correct configurations.  NOTE:  It is never necessary to disable options with kconfig-tweak.  If you enable a choice selection, it is smart enough to automatically disable the other options.  These greately reduces the size and complexity of sethost.sh.
   
   This change resolves issue #386 
   
   ## Impact
   
   sed edits do not handle all of the dependencies correct and generates invalid configurations.  Most defconfig files specify Linux by default, so you will only see the effect of the corruped configuration when sethost changes changes to a different configuration.  Then, when 'make olddefconfig' is subsequentyly run, the corruption in the defconfig file is reflected by warnings such as:
   
      $ tools/configure.sh -c stm32f4discovery:nsh
         Copy files
         Select CONFIG_HOST_WINDOWS=y
         Select CONFIG_WINDOWS_CYGWIN=y
         Refreshing...
       .config:62:warning: override: reassigning to symbol HOST_WINDOWS
       .config:62:warning: override: HOST_WINDOWS changes choice state
   
   Those warnings are eliminated by this changed.
   
   ## Testing
   
   Tested by repeatedly doing:
   
     tools/configure.sh -c stm32f4discovery:nsh
   


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620142463


   With further testing, I am seeing other issues:
   
   Now, with further testing, I see some problems:
   
       $ tools/configure.sh -l stm32f4discovery:nsh
         Copy files
         Select CONFIG_HOST_LINUX=y
         Refreshing...
       .config:62:warning: override: HOST_LINUX changes choice state
   
   I need to study further.  I think I may indeed have to disable features with kconfig-tweak.
   
   I just pushed a fix... lets check again.  The problem was that in windows defconfig files there were Windows environment options that were not being reset before 'make olddefconfig'.  If Linux or macOS are selected, it looks like we do have to explicitly disable Windows so that the windows enivoronment choices will not be a problem.  Not sure precisely why.


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620142463


   With further testing, I am seeing other issues:
   
   Now, with further testing, I see some problems:
   
       $ tools/configure.sh -l stm32f4discovery:nsh
         Copy files
         Select CONFIG_HOST_LINUX=y
         Refreshing...
       .config:62:warning: override: HOST_LINUX changes choice state
   
   I need to study further.  I think I may indeed have to disable features with kconfig-tweak.
   
   I just pushed a fix... lets check again.  The problem was that there were Windows environment options that were not being reset before 'make olddefconfig'.  If Linux or macOS are selected, it looks like we do have to explicitly disable Windows so that the windows enivoronment choices will not be a problem.  Not sure why.


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620167098


   @btashton @xiaoxiang781216 I am trying to test pr #897 but there is a problem with the macOS builds. When the kconfig-frontend package was build, it did not enable kconfig-tweak which is one of the kconfig-frontend utilities that must be built for the build to be successful.
   
   We cannot use sed. We must use kconfig-tweak!
   
   The problem is in Build / macOS (mips-riscv-x86-xtensa) (pull_request) :
   
   running CONFIG_SHELL=/bin/sh /bin/sh ./configure --prefix=/Users/runner/runners/2.169.1/work/incubator-nuttx/incubator-nuttx/sources/testing/../prebuilt/kconfig-frontends --disable-kconfig --disable-nconf --disable-qconf --disable-gconf --disable-mconf --disable-static --disable-shared --disable-L10n --disable-utils --no-create --no-recursion
   
   --disable-utils suppresses building of all kconfig-frontend utils.  That is a very bad idea.  For one thing, it suppresses kconfig-tweak and cause this problem. Just removing this should resolve the issues.
   


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620167098


   @btashton @xiaoxiang781216 I am trying to test pr #897 but there is a problem with the macOS builds. When the kconfig-frontend package was build, it did not enable kconfig-tweak which is one of the kconfig-frontend utilities that must be built for the build to be successful.
   
   We cannot use sed. We must use kconfig-tweak!
   
   The problem is in Build / macOS (mips-riscv-x86-xtensa) (pull_request) :
   
   running CONFIG_SHELL=/bin/sh /bin/sh ./configure --prefix=/Users/runner/runners/2.169.1/work/incubator-nuttx/incubator-nuttx/sources/testing/../prebuilt/kconfig-frontends --disable-kconfig --disable-nconf --disable-qconf --disable-gconf --disable-mconf --disable-static --disable-shared --disable-L10n --disable-utils --no-create --no-recursion
   
   --disable-utils suppresses building of all kconfig-frontend utils.  That is a very bad idea.  For one thing, it suppresses kconfig-tweak and cause this problem.
   


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620238293


   A logical flaw in the testing is that we do not have any defconfig files with MSYS as the Windows environment.  There could be undiscovered issues there.
   
   Another testing hole is the windows native build.  But they builds tend to be handled as special cases:  Because of the differences in the file system paths, you usually require some special interactions to get working correctly.  tools/sethost.sh -n will probably not work in the general case.


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620167098


   @btashton @xiaoxiang781216 I am trying to test pr #897 but there is a problem with the macOS builds. When the kconfig-frontend package was build, it did not enable kconfig-tweak which is one of the kconfig-frontend utilities that must be built for the build to be successful.
   
   We cannot use sed. We must use kconfig-tweak!


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620238293


   A logical flaw in the testing is that we do not have any defconfig files with MSYS as the Windows environment.  There could be undiscovered issues there.
   
   Another testing hole is the windows native build.  But those builds tend to be handled as special cases:  Because of the differences in the file system paths, you usually need to do some special interactions to get the configurations working correctly.  tools/sethost.sh -n will probably not work in the general case.


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620167098


   @btashton @xiaoxiang781216 I am trying to test pr #897 but there is a problem with the macOS builds. When the kconfig-frontend package was build, it did not enable kconfig-tweak which is one of the kconfig-frontend utilities that must be built for the build to be successful.
   
   We cannot use sed. We must use kconfig-tweak!
   
   The problem is in Build / macOS (mips-riscv-x86-xtensa) (pull_request) :
   
   running CONFIG_SHELL=/bin/sh /bin/sh ./configure --prefix=/Users/runner/runners/2.169.1/work/incubator-nuttx/incubator-nuttx/sources/testing/../prebuilt/kconfig-frontends --disable-kconfig --disable-nconf --disable-qconf --disable-gconf --disable-mconf --disable-static --disable-shared --disable-L10n --disable-utils --no-create --no-recursion
   
   --disable-utils suppresses building of all kconfig-frontend utils.  That is a very bad idea.  For one thing, it suppresses kconfig-tweak and cause this problem. Just removing this should resolve the issues.
   
   This addressed to incubator-nuttx-testing PR 38.  The checks for this PR depend on that PR 38.


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620142463


   With further testing, I am seeing other issues:
   
   Now, with further testing, I see some problems:
   
       $ tools/configure.sh -l stm32f4discovery:nsh
         Copy files
         Select CONFIG_HOST_LINUX=y
         Refreshing...
       .config:62:warning: override: HOST_LINUX changes choice state
   
   I need to study further.  I think I may indeed have to disable features with kconfig-tweak.
   
   I just pushed a fix... lets check again


----------------------------------------------------------------
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 #897: tools/sethost.sh: Correct error in setting a different host.

Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #897:
URL: https://github.com/apache/incubator-nuttx/pull/897#issuecomment-620142463


   With further testing, I am seeing other issues:
   
   Now, with further testing, I see some problems:
   
       $ tools/configure.sh -l stm32f4discovery:nsh
         Copy files
         Select CONFIG_HOST_LINUX=y
         Refreshing...
       .config:62:warning: override: HOST_LINUX changes choice state
   
   I need to study further.  I think I may indeed have to disable features with kconfig-tweak.
   
   


----------------------------------------------------------------
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