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/09/22 18:00:21 UTC
[GitHub] [incubator-nuttx] patacongo opened a new pull request #1871: Fix Cygwin build broken by commit 34b34e2d451
patacongo opened a new pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871
## Summary
Commit 34b34e2d451 uses the full path to libapps.a and introduced the use of the Make notdir command. That command breaks the Cygwin build because when a native Windows toolchain is used, the full path to libapps.a is a Windows-sytle path and the Make notdir command (like most other GNU Make commands) fails if it is passed a Windows-style path. This commit uses cygpath to convert the libapps.a path back to a POSIX-style path which can be handled by notdir, restoring full functionality for the Cygwin build.
NOTE to reviewers: The change looks weird because the logic is fully duplicated when the Windows tools is used while in reality only a small part of the logic changed. This was necessary because Make conditional logic does not work within the define. Apparently, the ifeq arguments are fully expanded before the definition is used. This results in make faioures like: Syntax error 'if (y,y).
## Impact
There should be no impacts to other build environments.
## Testing
On Cygwin:
$ make distclean
$ tools/configure.sh -c stm32f4discovery:nsh
$make
and PR checks for the other platforms.
----------------------------------------------------------------
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 #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696959571
@v01d Thanks for the merge
There are many other places where dir or notdir are used with paths. That is not a problem if the path can only be a POSIX path, or a relative path. Make-related paths are always posix-like with forward slashes. The only error case is when the path is passed to a Windows native toolchain (such as the ARM embedded toolchain for Windows). In that case, the paths will be Windows paths with C: for the volume and backslashes for delimiters.
There may be other cases where there are lurking failures in other configurations if they are run under Cygwin (which is all POSIX) with such a native toolchain (which is not POSIX). Grep-ing these I don't see any glaring issues, but they are somewhat difficult to interpret from only the grep results.
----------------------------------------------------------------
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 #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696897998
I think I make be able to remove the cygpath call by using the Bash basename vs. the make notdir command. Let me try that before merging.
----------------------------------------------------------------
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] v01d merged pull request #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
v01d merged pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871
----------------------------------------------------------------
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] v01d commented on pull request #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696915857
LGTM
Note that there's no difference between curly braces and parenthesis.
----------------------------------------------------------------
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] v01d merged pull request #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
v01d merged pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871
----------------------------------------------------------------
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] v01d commented on pull request #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696915857
LGTM
Note that there's no difference between curly braces and parenthesis.
----------------------------------------------------------------
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 #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696912434
>
>
> I think I make be able to remove the cygpath call by using the Bash basename vs. the make notdir command. Let me try that before merging.
@v01d Okay this seems to work too and is the cleaner solution. Ready for review.
I just replaced the Make notdir command which cannot handle Windows paths with the Bash basename command which can.
----------------------------------------------------------------
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 #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696926604
This is essentially the same build breakage as PR #1682: Changing from the Bash dirname command to the make dir command broke the Cygwin build because the make dir command cannot handle Windows paths. And the same fix: In that case, the make dir command was replaced with the more robust Bash dirname command.
----------------------------------------------------------------
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 #1871: Fix Cygwin build broken by commit 34b34e2d451
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1871:
URL: https://github.com/apache/incubator-nuttx/pull/1871#issuecomment-696897998
----------------------------------------------------------------
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