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/09/05 23:49:07 UTC
[GitHub] [incubator-nuttx] yamt opened a new pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
yamt opened a new pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477
## Summary
This reverts commit b05737d78fb0e746f46a19f2cdd9d4dc59e54985.
Because it broke clang-based builds.
## 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
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913951848
> > @yamt, since #4451 is related to #3836. I would propose to revert them in batch. Also, it's better to make a demo how to enable lto without gcc driver.
>
> lto thing has been on my low-priority todo for a while. if anyone can beat me, it's welcome.
>
> otoh, this one is more critical as it's breaking my (private) ci and workflows right now.
>
> if you are going to merge #3836 without waiting for "lto demo", it's fine for me. (IMO it isn't reasonable to require me the demo in the first place.)
It's fine without lto demo.
> otherwise, let's fix this independently from #3836 .
Ok, let's merge this PR first.
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913367458
> > ## Summary
> > This reverts commit [b05737d](https://github.com/apache/incubator-nuttx/commit/b05737d78fb0e746f46a19f2cdd9d4dc59e54985).
> > Because it broke clang-based builds.
>
> @yamt could you point out which config break? https://github.com/apache/incubator-nuttx/pull/4451/checks show that the change pass CI on macOS.
clang build of sim/linux at least.
i don't think we are testing it on the ci.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913473840
> after thinking a bit, i guess the problem is not only for sim.
> for example, it shouldn't be used in case of CONFIG_ARMV7M_TOOLCHAIN=CLANG, right?
> i prefer this to be reverted for now unless we can fix it quickly.
> (i don't think i can fix and test non-sim bits so quickly)
Fix by https://github.com/apache/incubator-nuttx/pull/4482
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913488167
@yamt, since https://github.com/apache/incubator-nuttx/pull/4451 is related to https://github.com/apache/incubator-nuttx/pull/3836. I would propose to revert them in batch. Also, it's better to make a demo how to enable lto without gcc driver.
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913444783
> > > i suppose LTO build will be an optional config.
> > > i guess it makes more sense to make these conditional on, say, CONFIG_GCC_LTO.
> >
> >
> > Could be, but [b05737d](https://github.com/apache/incubator-nuttx/commit/b05737d78fb0e746f46a19f2cdd9d4dc59e54985) doesn't really enable lto, but make enable lto more easier.
> > > how do you think?
> >
> >
> > For sim clang support, the better fix is SIM_TOOLCHAIN_CLANG/SIM_TOOLCHAIN_GCC and arch/sim/Toolchain.defs.
>
> i'm not sure it's better or not. but i agree it's consistent with the other boards.
> are you going to fix this that way?
or are you asking me to do it? either way is ok for me.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913951848
Ok, let's merge this PR first.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913411666
> i suppose LTO build will be an optional config.
> i guess it makes more sense to make these conditional on, say, CONFIG_GCC_LTO.
Could be, b05737d doesn't really enable lto, but make enable lto more easier.
> how do you think?
For sim clang support, the better fix is SIM_TOOLCHAIN_CLANG/SIM_TOOLCHAIN_GCC and arch/sim/Toolchain.defs.
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913419056
> > i suppose LTO build will be an optional config.
> > i guess it makes more sense to make these conditional on, say, CONFIG_GCC_LTO.
>
> Could be, but [b05737d](https://github.com/apache/incubator-nuttx/commit/b05737d78fb0e746f46a19f2cdd9d4dc59e54985) doesn't really enable lto, but make enable lto more easier.
>
> > how do you think?
>
> For sim clang support, the better fix is SIM_TOOLCHAIN_CLANG/SIM_TOOLCHAIN_GCC and arch/sim/Toolchain.defs.
i'm not sure it's better or not. but i agree it's consistent with the other boards.
are you going to fix this that way?
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913369885
i suppose LTO build will be an optional config.
i guess it makes more sense to make these conditional on, say, CONFIG_GCC_LTO.
how do you think?
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913449285
We can add an issue. The change isn't huge if you don't have free time, I can do it in this week.
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913917718
> @yamt, since #4451 is related to #3836. I would propose to revert them in batch. Also, it's better to make a demo how to enable lto without gcc driver.
lto thing has been on my low-priority todo for a while. if anyone can beat me, it's welcome.
otoh, this one is more critical as it's breaking my (private) ci and workflows right now.
if you are going to merge https://github.com/apache/incubator-nuttx/pull/3836 without waiting for "lto demo", it's fine for me. (IMO it isn't reasonable to require me the demo in the first place.)
otherwise, let's fix this independently from https://github.com/apache/incubator-nuttx/pull/3836 .
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913488167
@yamt, since https://github.com/apache/incubator-nuttx/pull/4451 is related to https://github.com/apache/incubator-nuttx/pull/3836. I would propose revert them in batch. Also, it's better to make a demo how to enable lto without gcc driver.
--
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
[GitHub] [incubator-nuttx] yamt commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913457727
after thinking a bit, i guess the problem is not only for sim.
for example, it shouldn't be used in case of CONFIG_ARMV7M_TOOLCHAIN=CLANG, right?
i prefer this to be reverted for now unless we can fix it quickly.
(i don't think i can fix and test non-sim bits so quickly)
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913315877
> ## Summary
> This reverts commit [b05737d](https://github.com/apache/incubator-nuttx/commit/b05737d78fb0e746f46a19f2cdd9d4dc59e54985).
>
> Because it broke clang-based builds.
>
@yamt could you point out which config break? https://github.com/apache/incubator-nuttx/pull/4451/checks show that the change pass CI on macOS.
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913473840
Fix https://github.com/apache/incubator-nuttx/pull/4482
--
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
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #4477: Revert "arch: Replace ar and nm with gcc-ar and gcc-nm"
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #4477:
URL: https://github.com/apache/incubator-nuttx/pull/4477#issuecomment-913411666
> i suppose LTO build will be an optional config.
> i guess it makes more sense to make these conditional on, say, CONFIG_GCC_LTO.
Could be, but b05737d doesn't really enable lto, but make enable lto more easier.
> how do you think?
For sim clang support, the better fix is SIM_TOOLCHAIN_CLANG/SIM_TOOLCHAIN_GCC and arch/sim/Toolchain.defs.
--
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