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 2022/01/29 23:47:51 UTC

[GitHub] [incubator-nuttx] AlanRosenthal opened a new pull request #5375: Generate $(SCRIPTOUT) during the build phase

AlanRosenthal opened a new pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375


   ## Summary
   The only use of the boards::context was to generate $(SCRIPTOUT) that is used by the linker.
   
   Instead of generating it at context phase, generate it via normal dependency specification.
   This PR also removes the board::context rule, as it is no longer required.
   
   ## Impact
   This work is part of the effort in #5205, which will clean up the context generation phase.
   
   ## Testing
   Verified build locally


-- 
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] pkarashchenko commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r806215491



##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       I would like to test this change on samv7 in prior to merging. Currently there is some flexibility to auto-generate linker scripts depending on ROM loader vs MCUboot loader and I feel that we might miss this functionality with this change.

##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       Thank you. Let's proceed with https://github.com/apache/incubator-nuttx/pull/5496 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] AlanRosenthal commented on pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#issuecomment-1039572920


   To make this PR easier to review, I moved a lot of the changes to #5496


-- 
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] AlanRosenthal commented on pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#issuecomment-1051100395


   need to rebase/fix some issues, no need for review right now


-- 
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] pkarashchenko commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r806215491



##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       I would like to test this change on samv7 in prior to merging. Currently there is some flexibility to auto-generate linker scripts depending on ROM loader vs MCUboot loader and I feel that we might miss this functionality with this change.




-- 
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] pkarashchenko commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r806280931



##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       Thank you. Let's proceed with https://github.com/apache/incubator-nuttx/pull/5496 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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r796527841



##########
File path: boards/xtensa/esp32s3/esp32s3-devkit/src/Make.defs
##########
@@ -36,14 +36,12 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s3.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s3_out.ld
 
-.PHONY = context distclean
+.PHONY = distclean

Review comment:
       or remove this line




-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r806261705



##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       The goal of this PR was to change how the files were generated (using dependency specification rather than at the beginning with the context rule). If something is changing then we definitely need to address that. It will be easier to view a diff of this PR after #5496 is merged




-- 
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] AlanRosenthal commented on pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#issuecomment-1039572920


   To make this PR easier to review, I moved a lot of the changes to #5496


-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r799016094



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       Definitely open to suggestions to make it more straightforward.
   Currently the rules for `SCRIPTOUT` is defined in `arch/arm/<arch-name>/board/board/Make.defs` aka `$(BOARD_DIR)/src/Make.defs`
   `arch/<arch-name>/src/board/Makefile` includes `board/Make.defs`. Right now the way I'm generating `$(SCRIPTOUT)` is by essentially calling `make -C board $(SCRIPTOUT)`, because that Makefile knows how to create the file.
   
   I could modify `arch/<arch-name>/src/Makefile` to `include /board/board/Make.defs` and then add the dependency to `nuttx$(EXEEXT)`
   
   @xiaoxiang781216 is that what you had in mind or did you have a different idea?




-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r806261705



##########
File path: boards/arm/samv7/common/scripts/flat.memory
##########
@@ -22,10 +22,6 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
-  ARCHSCRIPT = -T "${shell cygpath -w $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld}"
-else
-  ARCHSCRIPT = -T$(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)samv7.ld
-endif
+ARCHSCRIPT_PREPROCESS = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)flat-template.ld

Review comment:
       The goal of this PR was to change how the files were generated (using dependency specification rather than at the beginning with the context rule). If something is changing then we definitely need to address that. It will be easier to view a diff of this PR after #5496 is merged




-- 
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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r796510216



##########
File path: boards/xtensa/esp32s3/esp32s3-devkit/src/Make.defs
##########
@@ -36,14 +36,12 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s3.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s3_out.ld
 
-.PHONY = context distclean
+.PHONY = distclean

Review comment:
       ```suggestion
   .PHONY = clean
   ```




-- 
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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r796529316



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       how about we make the link script preprocess as a common step and put into to arch/**/Makefile?




-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r799016094



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       So it's a bit confusing how it currently is defined. Definitely open to suggestions to make it more straightforward.
   Currently the rules for `SCRIPTOUT` is defined in `arch/arm/<arch-name>/board/board/Make.defs` aka `$(BOARD_DIR)/src/Make.defs`
   `arch/<arch-name>/src/board/Makefile` includes `board/Make.defs`. Right now the way I'm generating `$(SCRIPTOUT)` is by essentially calling `make -C board $(SCRIPTOUT)`, because that Makefile knows how to create the file.
   
   I could modify `arch/<arch-name>/src/Makefile` to `include /board/board/Make.defs` and then add the dependency to `nuttx$(EXEEXT)`
   
   @xiaoxiang781216 is that what you had in mind or did you have a different idea?




-- 
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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r800090845



##########
File path: arch/arm/src/Makefile
##########
@@ -151,6 +151,11 @@ $(KBIN): $(OBJS)
 board$(DELIM)libboard$(LIBEXT):
 	$(Q) $(MAKE) -C board libboard$(LIBEXT) EXTRAFLAGS="$(EXTRAFLAGS)"
 
+ifneq ($(SCRIPTOUT),)
+$(SCRIPTOUT): $(LDSCRIPT_TEMPLATE)
+	$(Q) $(CC) -isystem $(TOPDIR)/include -I $(BOARD_COMMON_DIR)$(DELIM)scripts -C -P -x c -E $(LDSCRIPT_TEMPLATE) -o $@

Review comment:
       can we call PREPROCESS instead

##########
File path: boards/risc-v/esp32c3/esp32c3-devkit/scripts/Make.defs
##########
@@ -23,7 +23,15 @@ include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/tools/esp32c3/Config.mk
 include $(TOPDIR)/arch/risc-v/src/common/Toolchain.defs
 
+ifneq ($(wildcard $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld),)
+  LDSCRIPT_TEMPLATE = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld
+else
+  LDSCRIPT_TEMPLATE = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld
+endif
+
 LDSCRIPT1 = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3_out.ld
+SCRIPTOUT = $(LDSCRIPT1)

Review comment:
       How about we remove -T from ARCHSCRIPT:
   ```
   ifneq ($(wildcard $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld),)
     LDSCRIPT1 = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld
   else
     LDSCRIPT1 = $(BOARD_COMMON_DIR)$(DELIM)scripts$(DELIM)esp32c3.template.ld
   endif
   
   ifeq ($(CONFIG_ESP32C3_APP_FORMAT_MCUBOOT),y)
     LDSCRIPT2 = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3_mcuboot.ld
   else
     LDSCRIPT2 = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3.ld
   endif
   LDSCRIPT3 = $(BOARD_DIR)$(DELIM)scripts$(DELIM)esp32c3_rom.ld
   
   ifeq ($(CONFIG_CYGWIN_WINTOOL),y)
     ARCHSCRIPT = "${shell cygpath -w $(LDSCRIPT1)}"
     ARCHSCRIPT += "${shell cygpath -w $(LDSCRIPT2)}"
     ARCHSCRIPT += "${shell cygpath -w $(LDSCRIPT3)}"
   else
     ARCHSCRIPT = $(LDSCRIPT1) $(LDSCRIPT2) $(LDSCRIPT3)
   endif
   ```
   and always preprocess files contained in ARCHSCRIPT. Finally,  add back -T before append it to LDFLAGS.




-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r795105453



##########
File path: boards/Makefile
##########
@@ -74,15 +74,7 @@ makedepfile: $(CSRCS:.c=.ddc) $(ASRCS:.S=.dds) $(CXXSRCS:.cxx=.ddx)
 
 depend: .depend
 
-context:
-ifeq ($(BOARD_INSTALLED),y)
-	$(Q) $(MAKE) -C $(BOARDDIR) context
-endif
-
-clean_context:
-
-clean: clean_context
-	$(call DELFILE, $(BIN))

Review comment:
       BIN is also deleted as part of the CLEAN macro




-- 
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] AlanRosenthal commented on a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r799016094



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       Definitely open to suggestions to make it more straightforward.
   Currently the rules for `SCRIPTOUT` is defined in `arch/arm/<arch-name>/board/board/Make.defs` aka `$(BOARD_DIR)/src/Make.defs`
   `arch/<arch-name>/src/board/Makefile` includes `board/Make.defs`. Right now the way I'm generating `$(SCRIPTOUT)` is by essentially calling `make -C board $(SCRIPTOUT)`, because that Makefile knows how to create the file.
   
   I could modify `arch/<arch-name>/src/Makefile` to `include /board/board/Make.defs` and then add  `SCRIPTOUT` as a dependency to `nuttx$(EXEEXT)`
   
   @xiaoxiang781216 is that what you had in mind or did you have a different idea?




-- 
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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r796529316



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       how about we let the link script preprocess as a common step and put into to arch/**/Makefile?




-- 
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 a change in pull request #5375: Generate $(SCRIPTOUT) during the build phase

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5375:
URL: https://github.com/apache/incubator-nuttx/pull/5375#discussion_r799278107



##########
File path: boards/xtensa/esp32s2/esp32s2-saola-1/src/Make.defs
##########
@@ -44,14 +44,10 @@ endif
 SCRIPTIN = $(SCRIPTDIR)$(DELIM)esp32s2.template.ld
 SCRIPTOUT = $(SCRIPTDIR)$(DELIM)esp32s2_out.ld
 
-.PHONY = context distclean
-
 $(SCRIPTOUT): $(SCRIPTIN) $(CONFIGFILE)

Review comment:
       > Definitely open to suggestions to make it more straightforward. Currently the rules for `SCRIPTOUT` is defined in `arch/arm/<arch-name>/board/board/Make.defs` aka `$(BOARD_DIR)/src/Make.defs` `arch/<arch-name>/src/board/Makefile` includes `board/Make.defs`. Right now the way I'm generating `$(SCRIPTOUT)` is by essentially calling `make -C board $(SCRIPTOUT)`, because that Makefile knows how to create the file.
   > 
   > I could modify `arch/<arch-name>/src/Makefile` to `include /board/board/Make.defs` and then add `SCRIPTOUT` as a dependency to `nuttx$(EXEEXT)`
   
   I think it's a common request to preprocess the linker script before pass it to the linker. @anchao made a patch before:
   https://github.com/FishsemiCode/nuttx/commit/dcf70e6142308724473ae895f8c2d01cf5e220d3
   But, it isn't upstream yet.
   
   > 
   > @xiaoxiang781216 is that what you had in mind or did you have a different idea?
   
   My suggestion is:
   
   1. Make the linker script preprocess as the common step
   2. Put this rule to arch/*/src/Makefile
   3. Remove the linker script preprocess from board's Make.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