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 2020/09/19 15:26:34 UTC

[GitHub] [incubator-nuttx-apps] v01d opened a new pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

v01d opened a new pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397

## Summary

Before this change CLEANDIRS was used, which is built from
detecting dummy .depend/.kconfig files, which is problematic
since when there's an intermediate subdirectory which does not
have it, subdirectories below this level will not be found
(even if they do have these files).

Since we already know exactly which subdirectories we need
to clean based on configured apps, we directly clean over these.

I started looking into this since I found that the `btsak` application
was not being cleaned due to this reason, which sits at `apps/wireless/bluetooth/btsak`
and `bluetooth` was not being cleaned since there were none of this dummy files there.

I would like to hear your thoughts on why this might be wrong
or not. I feel there's quite a bit of "magic files" placed around and
these are used to drive many targets and this is becoming very confusing
to maintain. My reasoning is that we should go into the subdirectories
we know need cleaning from the top-level Makefile directly and then let
each application Makefile drive the cleaning using its `clean` target.

I have not changed yet the similar logic in `Directory.mk` for `CLEANSUBDIRS`
which is still driven by `.depend` and `.kconfig` files as it is not clear to me
how works. I'm guessing that after this change, it is not necessary, but I'm unsure.

## Impact

Application build system

## Testing

Building and cleaning and looking for leftover `.o`s

----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d commented on pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d commented on pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397#issuecomment-695263624


   Note: an alternative approach would be to solve this recursively: have the top-level Makefile go into one level below, clean there, recurse into subdirectories. I tried this (and I believe this is how it was originally) but this is really slow because it goes into absolutely every subdirectory, even when we know it was not built. Thus my reasoning of directly going into configured application directories.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d commented on pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d commented on pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397#issuecomment-695271579


   Actually I believe the clean logic is Directory.mk is unnecessary since it is designed following the recursive approach. I removed the clean target there and all .o's were still removed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d commented on pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d commented on pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397#issuecomment-695321034


   I see this is problematic because we need to clean autogenerated Kconfig files in intermediate directories. I'm going to try another approach: to use CONFIGURED_APPS to do go through all intermediate subdirectories of CONFIGURED_APPS only


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d commented on pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d commented on pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397#issuecomment-695279244


   One final note (sorry for spamming with comments): I understand this may leave `.o` present if you for example build, configure an app out and then clean. But I think that: a) this should never be a problem to build with extra `.o`s present as all the logic looks for specific `.o`s, not just any `.o` in a directory, and b) if one wants a pristine directory it should do `distclean`. In this case `distclean` should indeed go through **every** subdirectory to ensure a clean workspace. But that's at least my thinking.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d closed pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d closed pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-nuttx-apps] v01d commented on pull request #397: apps: use CONFIGURED_APPS to iterate clean targets

Posted by GitBox <gi...@apache.org>.

v01d commented on pull request #397:
URL: https://github.com/apache/incubator-nuttx-apps/pull/397#issuecomment-695332436


   I'm closing this since since I explained the problem in the linked issue but I don't yet have a successful solution


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org