You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "cuiziwei1 (via GitHub)" <gi...@apache.org> on 2024/10/12 07:31:56 UTC
[PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
cuiziwei1 opened a new pull request, #14159:
URL: https://github.com/apache/nuttx/pull/14159
## Summary
## Impact
## Testing
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "GUIDINGLI (via GitHub)" <gi...@apache.org>.
GUIDINGLI merged PR #14159:
URL: https://github.com/apache/nuttx/pull/14159
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on code in PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#discussion_r1798614954
##########
libs/libxx/libcxx.cmake:
##########
@@ -95,14 +95,6 @@ list(APPEND SRCS ${SRCSTMP})
set(FLAGS -Wno-attributes -Wno-deprecated-declarations -Wno-shadow
-Wno-sign-compare)
-if(NOT DEFINED GCCVER)
- execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version
- OUTPUT_VARIABLE GCC_VERSION_OUTPUT)
- string(REGEX MATCH "\\+\\+.* ([0-9]+)\\.[0-9]+" GCC_VERSION_REGEX
- "${GCC_VERSION_OUTPUT}")
- set(GCCVER ${CMAKE_MATCH_1})
-endif()
-
if(GCCVER EQUAL 12)
Review Comment:
Should this be greater equal?
##########
arch/arm/src/cmake/gcc.cmake:
##########
@@ -66,7 +66,9 @@ execute_process(COMMAND ${CMAKE_C_COMPILER} --version
OUTPUT_VARIABLE GCC_VERSION_OUTPUT)
string(REGEX MATCH "\\+\\+.* ([0-9]+)\\.[0-9]+" GCC_VERSION_REGEX
"${GCC_VERSION_OUTPUT}")
-set(GCCVER ${CMAKE_MATCH_1})
+set(GCCVER
+ ${CMAKE_MATCH_1}
+ PARENT_SCOPE)
if(GCCVER EQUAL 12)
Review Comment:
Should this also be greater equal?
##########
arch/arm/src/common/Toolchain.defs:
##########
@@ -309,10 +309,6 @@ else
# Wrong warning array subscript [0] is outside array bounds:
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523
- ifeq ($(GCCVER),)
- export GCCVER := $(shell $(CC) --version | grep gcc | sed -E "s/.* ([0-9]+\.[0-9]+).*/\1/" | cut -d'.' -f1)
- endif
-
ifeq ($(GCCVER),12)
Review Comment:
Should this be `($(shell expr $(GCCVER) \>= 12), 1)`?
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "cuiziwei1 (via GitHub)" <gi...@apache.org>.
cuiziwei1 commented on PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#issuecomment-2408446858
@anchao @xiaoxiang781216 please review this patch to resolve https://github.com/apache/nuttx/pull/14130#issuecomment-2408282192
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "xuxin930 (via GitHub)" <gi...@apache.org>.
xuxin930 commented on code in PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#discussion_r1797626983
##########
tools/Config.mk:
##########
@@ -732,6 +732,9 @@ export DEFINE_PREFIX ?= $(subst X,,${shell $(DEFINE) "$(CC)" X 2> ${EMPTYFILE}})
export INCDIR_PREFIX ?= $(subst "X",,${shell $(INCDIR) "$(CC)" X 2> ${EMPTYFILE}})
export INCSYSDIR_PREFIX ?= $(subst "X",,${shell $(INCDIR) -s "$(CC)" X 2> ${EMPTYFILE}})
+# Get the GCC version number
+export GCCVER = $(shell $(CXX) --version | grep ++ | sed -E 's/.* ([0-9]+\.[0-9]+).*/\1/' | cut -d'.' -f1)
Review Comment:
```suggestion
ifeq ($(CONFIG_ARCH_TOOLCHAIN_GNU), y)
export GCCVER = $(shell $(CC) --version | grep ++ | sed -E 's/.* ([0-9]+\.[0-9]+).*/\1/' | cut -d'.' -f1)
endif
```
##########
arch/arm/src/cmake/gcc.cmake:
##########
@@ -62,7 +62,7 @@ endif()
# array subscript [0] is outside array bounds:
# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105523
-execute_process(COMMAND ${CMAKE_C_COMPILER} --version
+execute_process(COMMAND ${CMAKE_CXX_COMPILER} --version
Review Comment:
```suggestion
execute_process(COMMAND ${CMAKE_C_COMPILER} --version
OUTPUT_VARIABLE GCC_VERSION_OUTPUT)
string(REGEX MATCH "\\+\\+.* ([0-9]+)\\.[0-9]+" GCC_VERSION_REGEX
"${GCC_VERSION_OUTPUT}")
set(GCCVER ${CMAKE_MATCH_1} PARENT_SCOPE)
```
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "cuiziwei1 (via GitHub)" <gi...@apache.org>.
cuiziwei1 commented on code in PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#discussion_r1798822230
##########
arch/arm/src/cmake/gcc.cmake:
##########
@@ -66,7 +66,9 @@ execute_process(COMMAND ${CMAKE_C_COMPILER} --version
OUTPUT_VARIABLE GCC_VERSION_OUTPUT)
string(REGEX MATCH "\\+\\+.* ([0-9]+)\\.[0-9]+" GCC_VERSION_REGEX
"${GCC_VERSION_OUTPUT}")
-set(GCCVER ${CMAKE_MATCH_1})
+set(GCCVER
+ ${CMAKE_MATCH_1}
+ PARENT_SCOPE)
if(GCCVER EQUAL 12)
Review Comment:
ok, please review https://github.com/apache/nuttx/pull/14241
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#issuecomment-2408442787
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
I need the content of the PR to assess if it meets the NuttX requirements. Please provide the following:
* **The actual PR content:** This includes the PR description, code changes, and any related files.
* **Links to the PR:** If the PR is already open on GitHub, please share the link.
With this information, I can review the PR against the NuttX requirements and provide a concise assessment.
--
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
Re: [PR] Unify the definition of GCCVER and remove duplicate code. [nuttx]
Posted by "xuxin930 (via GitHub)" <gi...@apache.org>.
xuxin930 commented on PR #14159:
URL: https://github.com/apache/nuttx/pull/14159#issuecomment-2408447329
hi @anchao please review whether this location is reasonable.
--
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