Cygwin directories not being cleaned

[patacongo]

Under Cygwin, the following directories are not being removed by the distclean target:

arch/<arch>/include/board
arch/<arch>/include/chip

This was noted in apache/nuttx-testing#39

I have several old board and and chip directories left over from unsuccessful distclean operations on different architectures. These two lines in arch/arm/src/Makefile (and other arch Makefiles) are not apparently not working under Cygwin:

    $(Q) $(DIRUNLINK) include/arch/board
    $(Q) $(DIRUNLINK) include/arch/chip

Cygwin supports is own style of symbolic links and these are used EXCEPT for the case where a Windows native toolchain is used such as the popular (https://developer.arm.com/tools-and-software/open-source-software/developer-tools/gnu-toolchain/gnu-rm).

Those are Window native tools and cannot follow non-standard Cygwin symbolic links so in this case, the tools/dirlinks.sh script does a full directory copy and the corresponding unlink script does a directory removal.

Without knowing more, it looks like the DIRUNLINK is failing to remove the Cygwin symbolic link or the copied directory.

[patacongo]

I don't think this bug has been introduced recently. I have not build the ez80 (under arch/z80) in many months.

I will do this as part of the review for the 10.0.0 release.

[patacongo]

I hate the close button being associated with the comment window!!!!!!!!!!

[patacongo]

The stranded directories are NOT the directory copies, but Cygwin symbolic links:

$ ls -ld arch/arm/include/chip
lrwxrwxrwx 1 spuda spuda 83 Aug 30 10:09 arch/arm/include/chip -> /cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/arch/arm/include/stm32

When I do

make distclean
tools/configures.sh -c stm32f4discovery:nsh
make context

I get the expected copied directories (because I am using the Arm Embedded toolchain and CONFIG_CYGWIN_WINTOOL=y):

$ ls -ld include/arch
drwxrwxr-x+ 1 spuda spuda 0 Sep 23 08:43 include/arch

$ ls -ld include/arch/board
drwxrwxr-x+ 1 spuda spuda 0 Sep 23 08:43 include/arch/board

$ ls -ld include/arch/chip
ddrwxrwxr-x+ 1 spuda spuda 0 Sep 23 08:43 include/arch/chip

Bit I also get Cygwin Symbolic links:

$ ls -ld arch/arm/include/chip
lrwxrwxrwx 1 spuda spuda 83 Sep 23 08:42 arch/arm/include/chip -> /cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/arch/arm/include/stm32

$ ls -ld arch/arm/include/board
lrwxrwxrwx 1 spuda spuda 102 Sep 23 08:42 arch/arm/include/board -> /cygdrive/d/Spuda/Documents/projects/nuttx/master/nuttx_fork/boards/arm/stm32/stm32f4discovery/include

That should not be happening and exonerates the DIRUNLINK calls implicated in the Issue description. These symbolic links are being created erroneously sometime in the context target.

Doing 'make clean_context' does not remove any of those. That is an error. 'make clean_context' should undo everything that was done my 'make context'. Apparently, somewhere along the way that critical part of the clean_context target got moved to distclean, breaking the symmetry of the make targets.

When I do 'make distclean' the unexpected Cygwin symbolic links remain:

$ find . -name chip
./arch/arm/include/chip

$ find . -name board
./arch/arm/include/board
./boards/arm/stm32/common/board

[patacongo]

Aha... these bad directories are being created by configure.sh.

$ make distclean (and verify that there are no chip or board directories)
$ tools/configure.sh -c stm32f4discovery:nsh
$ find . -name board
./arch/arm/include/board
./arch/arm/src/board
./boards/arm/stm32/common/board
$ find . -name chip
./arch/arm/include/chip
./arch/arm/src/chip

@v01d @btashton This is a consequence of several changes to the Makefiles recently:

• The olddefconfig target used to invoke the clean_context target to remove all residuals from the configuration
• The symbolic links are (probably) created because CONFIG_CYGWIN_WINTOOL has not been selected yet in the configuration (it was initially a Linux configuration). The .config included by the Makefile is the original defconfig file and does not yet have the derived settings like CONFIG_CYGWIN_WINTOOL.=y. So the Makefile runs without sufficient information.
• The logic that removes these incorrect links was removed from its correct location in the clean_context target to the distclean target.

A possible fix would be to restore the 'make clean_context' and all reconfigurations to eliminate this garbage from running the Makefile with an incomplete configuration. The old, fully working logic in Makefile.unix was:

do_olddefconfig: dirlinks apps_preconfig
	$(Q) APPSDIR=${CONFIG_APPS_DIR} kconfig-conf --olddefconfig Kconfig

olddefconfig: do_olddefconfig clean_context

Where clean_context removed the bad settings left by the 'make olddefconfig' setup. Restoring this code would not fully work, however. You would also have to restore the correct behavior or clean_context since the key clean up was more to the distclean target.

These are the commits that broke the Cygwin build: b5dc837 and c709ed2 Parts of those changes need to be reverted.

I will never understand why people intentionally break full working, well tested code with no real justification.

@xiaoxiang781216 Can you discuss this with @liuguo09 and @anchao? It is their commits together that break the Cygwin build. I am hesitant to restore this necessary logic sense I cannot know their intent with these changes.

[patacongo]

@btashton @hartmannathan Should this be marked a blocker? I had it tagged that way, but decided not.

[hartmannathan]

I don't think it needs to be a blocker, but maybe Good First Issue?

…

On Wed, Sep 23, 2020 at 2:56 PM patacongo ***@***.***> wrote: @btashton <https://github.com/btashton> @hartmannathan <https://github.com/hartmannathan> Should this be marked a blocker? I had it tagged that way, but decided not. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1880 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AOD4O54Q2XGH7DSXDQEAP7TSHJAGXANCNFSM4RXCOPEQ> .

[btashton]

My inclination is for it to be a blocker because I cannot enable windows CI without it and I would like to see that pass prior to the releases.

[hartmannathan]

My inclination is for it to be a blocker because I cannot enable windows CI without it and I would like to see that pass prior to the releases.

Okay in that case, make it a blocker. With the Windows CI working, we will find out about breakage much more quickly. That will be a good thing.

[patacongo]

I don't think it needs to be a blocker, but maybe Good First Issue?

Way to complex and risky for a first issue. I don't even know how to solve it without coordination with Xiaomi guys. @xiaoxiang781216 I think we need to wait and at least get a recommendation (and preferably a draft PR). If you create the draft PR, I will check it out.