You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "W-M-R (via GitHub)" <gi...@apache.org> on 2024/11/04 12:22:32 UTC
[PR] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
W-M-R opened a new pull request, #14637:
URL: https://github.com/apache/nuttx/pull/14637
## Summary
makefile:
ifeq ($(CONFIG_BUILTIN_TOOLCHAIN),y)
COMPILER_RT_LIB = $(shell $(CC) $(ARCHCPUFLAGS) --print-libgcc-file-name)
ifeq ($(CONFIG_ARCH_TOOLCHAIN_CLANG),y)
ifeq ($(wildcard $(COMPILER_RT_LIB)),)
# if "--print-libgcc-file-name" unable to find the correct libgcc PATH
# then go ahead and try "--print-file-name"
COMPILER_RT_LIB := $(wildcard $(shell $(CC) $(ARCHCPUFLAGS) --print-file-name $(notdir $(COMPILER_RT_LIB))))
endif
endif
endif
## Impact
None
## Testing
None
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #14637:
URL: https://github.com/apache/nuttx/pull/14637
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #14637:
URL: https://github.com/apache/nuttx/pull/14637#issuecomment-2454773744
@W-M-R @lupyuen is it possible to implement a CI verification to raise an error when someone submit a commit that modify a Makefile/Make.defs but forget to update the CMakeLists.txt ?
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "nuttxpr (via GitHub)" <gi...@apache.org>.
nuttxpr commented on PR #14637:
URL: https://github.com/apache/nuttx/pull/14637#issuecomment-2454588401
[**\[Experimental Bot, please feedback here\]**](https://github.com/search?q=repo%3Aapache%2Fnuttx+13552&type=issues)
This PR does **not** fully meet the NuttX requirements. While it provides a summary of the code changes, it lacks crucial information in the Impact and Testing sections.
**Missing Information:**
* **Summary:** While the code change is shown, the *reason* for the change is missing. What problem does this fix? Why is this conditional compilation necessary? What was the previous behavior, and why was it incorrect?
* **Impact:** Claiming "None" across the board is unlikely. At minimum, this change affects the build process (using `--print-file-name` as a fallback). The PR needs to explain *why* this fallback is needed, under what circumstances it would be used, and what the implications are. Does this affect any specific architectures or configurations? Consider the impact on different toolchains (is this clang-specific?). Does this change have any potential downsides or corner cases? Even seemingly small changes can have unexpected consequences.
* **Testing:** "None" is unacceptable. The PR *must* demonstrate that the change works as intended and doesn't introduce regressions. This requires providing build logs and test results, both before and after the change, on at least one relevant platform. Ideally, tests should cover the specific scenario where the fallback logic is used. What happens if *both* `--print-libgcc-file-name` and `--print-file-name` fail? Is there proper error handling?
**Example of improved Impact section:**
Impact on build (will build process change)? YES. This change modifies the way `COMPILER_RT_LIB` is determined when using a built-in toolchain and Clang. Previously, only `--print-libgcc-file-name` was used. This change adds a fallback to use `--print-file-name` if the former fails to find the library. This fallback is necessary because [explain why the initial method fails in some cases]. This change is expected to affect builds using Clang on [list affected architectures/configurations]. It has no impact on builds using GCC or other compilers. There is a potential risk that `--print-file-name` might also fail, in which case [explain what happens and how it's handled - is there an error message, does the build fail, etc.?].
By providing this missing information, the PR will be much more likely to be accepted. Reviewers need to understand the context, rationale, and impact of the change to properly evaluate it. Thorough testing is essential to ensure the change is correct and doesn't introduce new problems.
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #14637:
URL: https://github.com/apache/nuttx/pull/14637#issuecomment-2454996530
If so, we have to build the same defconfig for both Makefile and CMake.
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #14637:
URL: https://github.com/apache/nuttx/pull/14637#issuecomment-2454932439
> > @W-M-R @lupyuen is it possible to implement a CI verification to raise an error when someone submit a commit that modify a Makefile/Make.defs but forget to update the CMakeLists.txt (or vice-versa) ?
>
> one method is switching half Makefile to CMake, which could make ci fast too.
I'm afraid it could impact some users, the agreement to add support to CMake was keep Makefile support together. To remove Makefiles we need to agree with people that were resistant to move to CMake.
--
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] clang/cmake: Fix clang cmake can't find libgcc, align with makefile [nuttx]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #14637:
URL: https://github.com/apache/nuttx/pull/14637#issuecomment-2454890028
> @W-M-R @lupyuen is it possible to implement a CI verification to raise an error when someone submit a commit that modify a Makefile/Make.defs but forget to update the CMakeLists.txt (or vice-versa) ?
one method is switching half Makefile to CMake, which could make ci fast too.
--
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