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/12/23 16:35:54 UTC
[GitHub] [incubator-nuttx] AlanRosenthal opened a new pull request #5069: Improve dependencies for `dirlinks`.
AlanRosenthal opened a new pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069
## Summary
This PR updates the dependencies for `dirlinks` so they're all real files. This allows `dirlinks` rule to not have to be rerun every time.
Changes:
* tools/link.sh
* link.sh now detects broken simlinks. Previously, it would return `0` if the simlink existed, but didn't point to anything.
* tools/Makefile.unix
* Added dependencies to simlinks as order-only prerequisites. See https://www.gnu.org/software/make/manual/html_node/Prerequisite-Types.html
* Removed `touch` from simlink recipes by specifying them as order only prerequisites.
* Check Kconfig variables before adding directories as simlinks
* Added rule for `$(TOPDIR)/arch/dummy/Kconfig`
* Added rule for `$(ARCH_SRC)/board/board`
* Removed `.clean_context` dependency from various simlinks. It doesn't make any sense why creating a simlink should require clearing the context, especially when `context` is dependent on `dirlinks`
* Added pattern rule (similar to `CONTEXTDIRS_DEPS`) for external folder dirlink dependencies
* Use $(APPDIR) instead of $(CONFIG_APPS_DIR), since on line 64 $(APPDIR) is validated and `realpath` is called on the path
* .gitignore
* Added ignore rule for external folder dirlink dependencies
## Impact
setting up the simlinks is a one time job, we shouldn't have to do it multiple times.
## Testing
Step 1: configure nuttx:
```
$ (cd tools && ./configure.sh -a ../incubator-nuttx-apps stm32f3discovery:nsh)
```
part of the configure step ends up calling `dirlinks`.
Step 2: We can confirm that `dirlinks` doesn't need to be run again by running with the question flag: `--question`
```
$ make dirlinks --question
$ echo $?
0
```
Step 3: confirm `make` succeeds.
```
make
```
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r775022426
##########
File path: tools/Makefile.unix
##########
@@ -524,9 +581,16 @@ config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf Kconfig
oldconfig: apps_preconfig
+ echo $@
Review comment:
left over debugging printfs, will remove
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r776573421
##########
File path: tools/Makefile.unix
##########
@@ -42,9 +42,13 @@ CONFIG_VERSION_BUILD ?= "0"
VERSION_ARG = -v $(CONFIG_VERSION_STRING) -b $(CONFIG_VERSION_BUILD)
else
-# Generate .version every time from GIT history
+# Generate .version.tmp every time from GIT history
+# Only update .version if the contents actually changes
+
+$(shell tools/version.sh .version.tmp)
+$(shell cp -u .version.tmp .version)
+# $(shell rm .version.tmp)
Review comment:
remove
##########
File path: tools/Makefile.unix
##########
@@ -42,9 +42,13 @@ CONFIG_VERSION_BUILD ?= "0"
VERSION_ARG = -v $(CONFIG_VERSION_STRING) -b $(CONFIG_VERSION_BUILD)
else
-# Generate .version every time from GIT history
+# Generate .version.tmp every time from GIT history
+# Only update .version if the contents actually changes
+
+$(shell tools/version.sh .version.tmp)
+$(shell cp -u .version.tmp .version)
Review comment:
why not use mv -u?
##########
File path: tools/Makefile.unix
##########
@@ -222,8 +226,11 @@ $(TOPDIR)/.version:
$(Q) tools/version.sh $(VERSION_ARG) .version
$(Q) chmod 755 .version
+testa: $(TOPDIR)/.version
Review comment:
remove
--
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] Ouss4 merged pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1004187553
> @AlanRosenthal let's merge into one patch?
done! happy new year
--
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] gustavonihei commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r774780549
##########
File path: tools/Makefile.unix
##########
@@ -524,9 +581,16 @@ config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf Kconfig
oldconfig: apps_preconfig
+ echo $@
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf --oldconfig Kconfig
-olddefconfig: apps_preconfig
+# olddefconfig first needs to clear the context, to ensure the configuration is up to date,
+# and no old artifacts (i.e. symlinks) still exist. By settings .NOTPARALLEL, we can enforce
+# order and ensure clean_context is executed first
Review comment:
```suggestion
# olddefconfig first needs to clear the context to ensure the configuration is up to date
# and no old artifacts (i.e. symlinks) still exist. By setting .NOTPARALLEL we can enforce
# order and ensure clean_context is executed first.
```
nit: Commas here seem unnecessary.
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1003376345
@AlanRosenthal let's merge into one patch?
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1003088968
sorry about that @gustavonihei & @Ouss4. When I rebased this PR I accidentally included WIP changes. I've reverted them
--
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] Ouss4 commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r776691122
##########
File path: tools/Makefile.unix
##########
@@ -42,9 +42,13 @@ CONFIG_VERSION_BUILD ?= "0"
VERSION_ARG = -v $(CONFIG_VERSION_STRING) -b $(CONFIG_VERSION_BUILD)
else
-# Generate .version every time from GIT history
+# Generate .version.tmp every time from GIT history
+# Only update .version if the contents actually changes
+
+$(shell tools/version.sh .version.tmp)
+$(shell cp -u .version.tmp .version)
Review comment:
I don't think the `-u` option is available for all platforms .
--
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] Ouss4 commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1000910462
> This PR needs to be tested with [apache/incubator-nuttx-apps#936](https://github.com/apache/incubator-nuttx-apps/pull/936). How do I configure CI to do that?
Unfortunately, we can't do that for non-release branches (at least, this is the state at the moment).
However, your apps PR is merged, so next push should pull the latest apps with the required changes.
--
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] acassis commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1002734519
> This PR needs to be tested with [apache/incubator-nuttx-apps#936](https://github.com/apache/incubator-nuttx-apps/pull/936). How do I configure CI to do that?
That PR was merged, so now it will be possible to test using it
--
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] gustavonihei commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r774779924
##########
File path: tools/Makefile.unix
##########
@@ -254,72 +254,140 @@ tools/cnvwindeps$(HOSTEXEEXT):
# Link the arch/<arch-name>/include directory to include/arch
-include/arch: .clean_context
- @echo "LN: include/arch to $(ARCH_DIR)/include"
- $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_DIR)/include include/arch
- $(Q) touch $@
+include/arch:
+ @echo "LN: $@ to $(ARCH_DIR)/include"
+ $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_DIR)/include $@
# Link the boards/<arch>/<chip>/<board>/include directory to include/arch/board
-include/arch/board: include/arch
- @echo "LN: include/arch/board to $(BOARD_DIR)/include"
- $(Q) $(DIRLINK) $(BOARD_DIR)/include include/arch/board
- $(Q) touch $@
+include/arch/board: | include/arch
+ @echo "LN: $@ to $(BOARD_DIR)/include"
+ $(Q) $(DIRLINK) $(BOARD_DIR)/include $@
-ifneq ($(BOARD_COMMON_DIR),)
# Link the boards/<arch>/<chip>/common dir to arch/<arch-name>/src/board
# Link the boards/<arch>/<chip>/<board>/src dir to arch/<arch-name>/src/board/board
-$(ARCH_SRC)/board: .clean_context
- @echo "LN: $(ARCH_SRC)/board to $(BOARD_COMMON_DIR)"
- $(Q) $(DIRLINK) $(BOARD_COMMON_DIR) $(ARCH_SRC)/board
- @echo "LN: $(ARCH_SRC)/board/board to $(BOARD_DIR)/src"
- $(Q) $(DIRLINK) $(BOARD_DIR)/src $(ARCH_SRC)/board/board
- $(Q) touch $@
+ifneq ($(BOARD_COMMON_DIR),)
+ARCH_SRC_BOARD_SYMLINK=$(BOARD_COMMON_DIR)
+ARCH_SRC_BOARD_BOARD_SYMLINK=$(BOARD_DIR)/src
else
-# Link the boards/<arch>/<chip>/<board>/src dir to arch/<arch-name>/src/board
+ARCH_SRC_BOARD_SYMLINK=$(BOARD_DIR)/src
+endif
-$(ARCH_SRC)/board: .clean_context
- @echo "LN: $(ARCH_SRC)/board to $(BOARD_DIR)/src"
- $(Q) $(DIRLINK) $(BOARD_DIR)/src $(ARCH_SRC)/board
- $(Q) touch $@
+ifneq ($(ARCH_SRC_BOARD_SYMLINK),)
+$(ARCH_SRC)/board:
+ @echo "LN: $@ to $(ARCH_SRC_BOARD_SYMLINK)"
+ $(Q) $(DIRLINK) $(ARCH_SRC_BOARD_SYMLINK) $@
+endif
+
+ifneq ($(ARCH_SRC_BOARD_BOARD_SYMLINK),)
+$(ARCH_SRC)/board/board: | $(ARCH_SRC)/board
+ @echo "LN: $@ to $(ARCH_SRC_BOARD_BOARD_SYMLINK)"
+ $(Q) $(DIRLINK) $(ARCH_SRC_BOARD_BOARD_SYMLINK) $@
endif
# Link the boards/<arch>/<chip>/drivers dir to drivers/platform
-drivers/platform: .clean_context
- @echo "LN: $(TOPDIR)/drivers/platform to $(BOARD_DRIVERS_DIR)"
- $(Q) $(DIRLINK) $(BOARD_DRIVERS_DIR) $(TOPDIR)/drivers/platform
- $(Q) touch $@
+drivers/platform:
+ @echo "LN: $@ to $(BOARD_DRIVERS_DIR)"
+ $(Q) $(DIRLINK) $(BOARD_DRIVERS_DIR) $@
# Link arch/<arch-name>/src/<chip-name> to arch/<arch-name>/src/chip
-$(ARCH_SRC)/chip: .clean_context
ifeq ($(CONFIG_ARCH_CHIP_CUSTOM),y)
- @echo "LN: $(ARCH_SRC)/chip to $(CHIP_DIR)"
- $(Q) $(DIRLINK) $(CHIP_DIR) $(ARCH_SRC)/chip
+ARCH_SRC_CHIP_SYMLINK_DIR=$(CHIP_DIR)
else ifneq ($(CONFIG_ARCH_CHIP),)
- @echo "LN: $(ARCH_SRC)/chip to $(ARCH_SRC)/$(CONFIG_ARCH_CHIP)"
- $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_SRC)/$(CONFIG_ARCH_CHIP) $(ARCH_SRC)/chip
+ARCH_SRC_CHIP_SYMLINK_DIR=$(TOPDIR)/$(ARCH_SRC)/$(CONFIG_ARCH_CHIP)
+endif
+
+ifneq ($(ARCH_SRC_CHIP_SYMLINK_DIR),)
+$(ARCH_SRC)/chip:
+ @echo "LN: $@ to $(ARCH_SRC_CHIP_SYMLINK_DIR)"
+ $(Q) $(DIRLINK) $(ARCH_SRC_CHIP_SYMLINK_DIR) $@
endif
- $(Q) cp -f $(CHIP_KCONFIG) $(TOPDIR)/arch/dummy/Kconfig
- $(Q) touch $@
# Link arch/<arch-name>/include/<chip-name> to include/arch/chip
-include/arch/chip: include/arch
ifeq ($(CONFIG_ARCH_CHIP_CUSTOM),y)
- @echo "LN: include/arch/chip to $(CHIP_DIR)/include"
- $(Q) $(DIRLINK) $(CHIP_DIR)/include include/arch/chip
+INCLUDE_ARCH_CHIP_SYMLINK_DIR=$(CHIP_DIR)/include
else ifneq ($(CONFIG_ARCH_CHIP),)
- @echo "LN: include/arch/chip to $(ARCH_INC)/$(CONFIG_ARCH_CHIP)"
- $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_INC)/$(CONFIG_ARCH_CHIP) include/arch/chip
+INCLUDE_ARCH_CHIP_SYMLINK_DIR=$(TOPDIR)/$(ARCH_INC)/$(CONFIG_ARCH_CHIP)
+endif
+
+ifneq ($(INCLUDE_ARCH_CHIP_SYMLINK_DIR),)
+include/arch/chip:
+ @echo "LN: $@ to $(INCLUDE_ARCH_CHIP_SYMLINK_DIR)"
+ $(DIRLINK) $(INCLUDE_ARCH_CHIP_SYMLINK_DIR) $@
+endif
+
+# Copy $(CHIP_KCONFIG) to arch/dummy/Kconfig
+
+arch/dummy/Kconfig:
+ @echo "CP: $@ to $(CHIP_KCONFIG)"
+ $(Q) cp -f $(CHIP_KCONFIG) $@
+
+DIRLINKS_SYMLINK = \
+ include/arch \
+ include/arch/board \
+ drivers/platform \
+
+DIRLINKS_FILE = \
+ arch/dummy/Kconfig \
+
+ifneq ($(INCLUDE_ARCH_CHIP_SYMLINK_DIR),)
+DIRLINKS_SYMLINK += include/arch/chip
+endif
+
+ifneq ($(ARCH_SRC_CHIP_SYMLINK_DIR),)
+DIRLINKS_SYMLINK += $(ARCH_SRC)/chip
endif
+
+ifneq ($(ARCH_SRC_BOARD_SYMLINK),)
+DIRLINKS_SYMLINK += $(ARCH_SRC)/board
+endif
+
+ifneq ($(ARCH_SRC_BOARD_BOARD_SYMLINK),)
+DIRLINKS_SYMLINK += $(ARCH_SRC)/board/board
+endif
+
+DIRLINKS_EXTERNAL_DIRS = boards
+
+ifneq ($(APPDIR),)
+DIRLINKS_EXTERNAL_DIRS += $(APPDIR)
+endif
+
+# Generate a pattern to build $(DIRLINKS_EXTERNAL_DIRS)
+
+DIRLINKS_EXERNAL_DEP = $(patsubst %,%/.dirlinks,$(DIRLINKS_EXTERNAL_DIRS))
+DIRLINKS_FILE += $(DIRLINKS_EXERNAL_DEP)
+
+dirlinks: $(DIRLINKS_FILE) | $(DIRLINKS_SYMLINK)
+
+# Pattern rule for $(DIRLINKS_EXERNAL_DEP)
Review comment:
```suggestion
DIRLINKS_EXTERNAL_DEP = $(patsubst %,%/.dirlinks,$(DIRLINKS_EXTERNAL_DIRS))
DIRLINKS_FILE += $(DIRLINKS_EXTERNAL_DEP)
dirlinks: $(DIRLINKS_FILE) | $(DIRLINKS_SYMLINK)
# Pattern rule for $(DIRLINKS_EXTERNAL_DEP)
```
Minor typo
--
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] gustavonihei commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r774780549
##########
File path: tools/Makefile.unix
##########
@@ -524,9 +581,16 @@ config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf Kconfig
oldconfig: apps_preconfig
+ echo $@
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf --oldconfig Kconfig
-olddefconfig: apps_preconfig
+# olddefconfig first needs to clear the context, to ensure the configuration is up to date,
+# and no old artifacts (i.e. symlinks) still exist. By settings .NOTPARALLEL, we can enforce
+# order and ensure clean_context is executed first
Review comment:
```suggestion
# olddefconfig first needs to clear the context to ensure the configuration is up to date
# and no old artifacts (i.e. symlinks) still exist. By setting .NOTPARALLEL we can enforce
# order and ensure clean_context is executed first.
```
nit: Commas here seem unnecessary. Also some other punctuation adjustments and a typo in `settings` (I 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 commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1004291329
@AlanRosenthal we may need update Makefile.win 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
[GitHub] [incubator-nuttx] gustavonihei commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1000505365
There is typo on `simlink` (correct should be **symlink**) spread throughout the PR.
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1000519229
> There is typo on `simlink` (correct should be **symlink**) spread throughout the PR.
whoops, will fix!
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r774858470
##########
File path: tools/Makefile.unix
##########
@@ -524,9 +581,16 @@ config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf Kconfig
oldconfig: apps_preconfig
+ echo $@
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf --oldconfig Kconfig
-olddefconfig: apps_preconfig
+# olddefconfig first needs to clear the context to ensure the configuration is up to date
+# and no old artifacts (i.e. symlinks) still exist. By setting .NOTPARALLEL we can enforce
+# order and ensure clean_context is executed first.
+
+.NOTPARALLEL: olddefconfig
+olddefconfig: clean_context apps_preconfig
+ echo $@
Review comment:
why need
##########
File path: tools/Makefile.unix
##########
@@ -524,9 +581,16 @@ config: apps_preconfig
$(Q) APPSDIR=${CONFIG_APPS_DIR} EXTERNALDIR=$(EXTERNALDIR) kconfig-conf Kconfig
oldconfig: apps_preconfig
+ echo $@
Review comment:
why need
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1000518838
This PR needs to be tested with https://github.com/apache/incubator-nuttx-apps/pull/936. How do I configure CI to do that?
--
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] Ouss4 commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1003374667
@xiaoxiang781216 @gustavonihei PTAL.
--
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] Ouss4 commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r776088779
##########
File path: tools/Makefile.unix
##########
@@ -246,80 +246,156 @@ tools/mkdeps$(HOSTEXEEXT):
tools/cnvwindeps$(HOSTEXEEXT):
$(Q) $(MAKE) -C tools -f Makefile.host cnvwindeps$(HOSTEXEEXT)
-# dirlinks, and helpers
+# .dirlinks, and helpers
#
# Directories links. Most of establishing the NuttX configuration involves
# setting up symbolic links with 'generic' directory names to specific,
# configured directories.
# Link the arch/<arch-name>/include directory to include/arch
-include/arch: .clean_context
- @echo "LN: include/arch to $(ARCH_DIR)/include"
- $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_DIR)/include include/arch
- $(Q) touch $@
+include/arch:
+ @echo "LN: $@ to $(ARCH_DIR)/include"
+ $(Q) $(DIRLINK) $(TOPDIR)/$(ARCH_DIR)/include $@
# Link the boards/<arch>/<chip>/<board>/include directory to include/arch/board
-include/arch/board: include/arch
- @echo "LN: include/arch/board to $(BOARD_DIR)/include"
- $(Q) $(DIRLINK) $(BOARD_DIR)/include include/arch/board
- $(Q) touch $@
+include/arch/board: | include/arch
+ @echo "LN: $@ to $(BOARD_DIR)/include"
+ $(Q) $(DIRLINK) $(BOARD_DIR)/include $@
-ifneq ($(BOARD_COMMON_DIR),)
# Link the boards/<arch>/<chip>/common dir to arch/<arch-name>/src/board
# Link the boards/<arch>/<chip>/<board>/src dir to arch/<arch-name>/src/board/board
-$(ARCH_SRC)/board: .clean_context
- @echo "LN: $(ARCH_SRC)/board to $(BOARD_COMMON_DIR)"
- $(Q) $(DIRLINK) $(BOARD_COMMON_DIR) $(ARCH_SRC)/board
- @echo "LN: $(ARCH_SRC)/board/board to $(BOARD_DIR)/src"
- $(Q) $(DIRLINK) $(BOARD_DIR)/src $(ARCH_SRC)/board/board
- $(Q) touch $@
+ifneq ($(BOARD_COMMON_DIR),)
+ARCH_SRC_BOARD_SYMLINK=$(BOARD_COMMON_DIR)
+ARCH_SRC_BOARD_BOARD_SYMLINK=$(BOARD_DIR)/src
else
-# Link the boards/<arch>/<chip>/<board>/src dir to arch/<arch-name>/src/board
+ARCH_SRC_BOARD_SYMLINK=$(BOARD_DIR)/src
+endif
-$(ARCH_SRC)/board: .clean_context
- @echo "LN: $(ARCH_SRC)/board to $(BOARD_DIR)/src"
- $(Q) $(DIRLINK) $(BOARD_DIR)/src $(ARCH_SRC)/board
- $(Q) touch $@
+ifneq ($(ARCH_SRC_BOARD_SYMLINK),)
+$(ARCH_SRC)/board:
+ @echo "LN: $@ to $(ARCH_SRC_BOARD_SYMLINK)"
+ $(Q) $(DIRLINK) $(ARCH_SRC_BOARD_SYMLINK) $@
+endif
+
+ifneq ($(ARCH_SRC_BOARD_BOARD_SYMLINK),)
+$(ARCH_SRC)/board/board: | $(ARCH_SRC)/board
+ @echo "LN: $@ to $(ARCH_SRC_BOARD_BOARD_SYMLINK)"
+ $(Q) $(DIRLINK) $(ARCH_SRC_BOARD_BOARD_SYMLINK) $@
endif
# Link the boards/<arch>/<chip>/drivers dir to drivers/platform
-drivers/platform: .clean_context
- @echo "LN: $(TOPDIR)/drivers/platform to $(BOARD_DRIVERS_DIR)"
- $(Q) $(DIRLINK) $(BOARD_DRIVERS_DIR) $(TOPDIR)/drivers/platform
- $(Q) touch $@
+drivers/platform:
+ @echo "LN: $@ to $(BOARD_DRIVERS_DIR)"
+ $(Q) $(DIRLINK) $(BOARD_DRIVERS_DIR) $@
Review comment:
Don't we need full path to link? Is it safe to remove `TOPDIR`?
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1002299496
> @AlanRosenthal we seem to have the same parallel build issue here as the last PR.
@Ouss4 CI is passing!
--
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] Ouss4 commented on a change in pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#discussion_r776697116
##########
File path: tools/Makefile.unix
##########
@@ -42,9 +42,13 @@ CONFIG_VERSION_BUILD ?= "0"
VERSION_ARG = -v $(CONFIG_VERSION_STRING) -b $(CONFIG_VERSION_BUILD)
else
-# Generate .version every time from GIT history
+# Generate .version.tmp every time from GIT history
+# Only update .version if the contents actually changes
+
+$(shell tools/version.sh .version.tmp)
+$(shell cp -u .version.tmp .version)
Review comment:
Actually the macOS build is already complaining about it: https://github.com/apache/incubator-nuttx/runs/4664246522?check_suite_focus=true#step:6:812
--
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 edited a comment on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal edited a comment on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1004187553
> @AlanRosenthal let's merge into one patch?
done! happy new year 🥳
--
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 #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
AlanRosenthal commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1002145682
> @AlanRosenthal we seem to have the same parallel build issue here as the last PR.
I believe something is wrong with the clean rule. I should have a patch fixing it up soon. Glad to see there's comprehensive tests in 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] Ouss4 commented on pull request #5069: Improve dependencies for `dirlinks`.
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #5069:
URL: https://github.com/apache/incubator-nuttx/pull/5069#issuecomment-1000941318
@AlanRosenthal we seem to have the same parallel build issue here as the last PR.
--
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