Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools/Makefile.unix: Remove FORBIDDEN CXD56 code in common Makefile. #442

Closed
patacongo opened this issue Mar 5, 2020 · 6 comments · Fixed by #457
Closed

tools/Makefile.unix: Remove FORBIDDEN CXD56 code in common Makefile. #442

patacongo opened this issue Mar 5, 2020 · 6 comments · Fixed by #457
Labels
modularity Needed to support modular architecture

Comments

@patacongo
Copy link
Contributor

Because of modularity concerns, this logic must be removed from tools/Makefile.unix:

483 ifeq ($(CONFIG_CXD56_BINARY),y)
484         $(Q) if [ ! -f "tools/cxd56/mkspk" ] ; then \
485                 echo ""; \ 
486                 echo "Please run the following command to build the needed tool"; \
487                 echo ""; \
488                 echo "cd tools/cxd56 && make && cd ../.."; \
489                 echo ""; \
490                 echo "run make again to create the nuttx.spk image."; \
491         else \
492                 echo "Generating: $(NUTTXNAME).spk"; \
493                 tools/cxd56/mkspk -c2 nuttx nuttx nuttx.spk; \
494         fi
495 endif

If is FORBIDDEN to include platform specific logic in common build logic.

@patacongo
Copy link
Contributor Author

I wonder if we don't need to hook in Makefile.unix/win to run:

$(TOPDIR)/tools/arch/Makefile at the conclusion of each build.

Where arch is a symbolic link to either a dummy directory or to a directory holding a platform-specific fixup?

@patacongo patacongo added the modularity Needed to support modular architecture label Mar 5, 2020
@patacongo
Copy link
Contributor Author

In many case, such as for the ESP32, such post build binary generation could be moved to /arch//src/Makefile. But this is not so convenient for other architectures like CXD56xx since /arch/arm/src/Makefile is shared by so many arm architectures.

So I think we need a systematic solution to handle all post-build binary generation that can work in a modular way for all platforms. There is a precedent for achitecture-specific logic in tools/xxxx/, so that would be a first thought for a location to provide such custom binary generation.

@patacongo
Copy link
Contributor Author

See related #437

I am thinking that a more systematic solution is needed both for CXD56xx and ESP32 and for any future platforms that require custom binary generation after the final linking step. I am thinking that perhaps we should support some Makefiles and tools in directories under tools/. Then perhaps we could do a wildcard check for:

(TOPDIR)/tools/$(CONFIG_ARCH)/Makefile

And if it exists, then run it. So in this case, it would check for tools/cxd56xx/Makefile and would executre tools/cxd56xx/Makefile to generate the binary using the changes currently in tools/Makefile.unix

This would serve the needs of all architectures, existing and future.

@xiaoxiang781216
Copy link
Contributor

See related #437

I am thinking that a more systematic solution is needed both for CXD56xx and ESP32 and for any future platforms that require custom binary generation after the final linking step. I am thinking that perhaps we should support some Makefiles and tools in directories under tools/. Then perhaps we could do a wildcard check for:

(TOPDIR)/tools/$(CONFIG_ARCH)/Makefile

Can we reuse the existing Makefile? For example:
arch\arm\src\cxd56xx\Make.defs to invoke chipset specific tool
arch\xtensa\src\Makefile to invoke arch specific tool
boards\arm\cxd56xx\spresense\scripts\Make.defs to invoke board specific tool
We just invoke some common target(e.g. postbuild) in the last step, then the above makefile could provide the hook as needed.

And if it exists, then run it. So in this case, it would check for tools/cxd56xx/Makefile and would executre tools/cxd56xx/Makefile to generate the binary using the changes currently in tools/Makefile.unix

This would serve the needs of all architectures, existing and future.

But tools has the flat directory structure, it's better to put the special tools or script to boards/arch directory:
1.boards/arm/xxxx/tools for board specific tool
2.arch/arm/xxxx/tools for chipset specific tool
3.arch/arm/armv7-a/tools for arch specific tool

@patacongo
Copy link
Contributor Author

Another option is to use a 'define POSTBUILD' in tools/Config.mk. That definition would be empty , but could be overriden in any Make.defs file by adding a 'define POSTBUILD'.

I do this for the ZDS-II tools which are not very compatible with GNU tools. I add a zds_Config.mk in tools/zds. That .mk file redefines several things in tools/Config.mk and tools/zds/zds_Config.mk is included by all Make.defs that use ZDS-II tools.

As a result, the very strange ZDS-II tools work with not impact to other build-related files.

@patacongo
Copy link
Contributor Author

PR #457 provide a resolution for this issue. Pleave review and, hopefully, merge.

cwespressif pushed a commit to cwespressif/incubator-nuttx that referenced this issue Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
modularity Needed to support modular architecture
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants