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

[RFC] tests: group applications in subdirectories #15358

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,14 @@ include $(RIOTMAKE)/boards.inc.mk
# Debug targets for build system migration
include $(RIOTMAKE)/dependencies_debug.inc.mk

# List all available applications
include $(RIOTMAKE)/app_dirs.inc.mk

ifeq (,$(filter $(APPDIR),$(APPLICATION_DIRS_ABSOLUTE)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be screaming in the face of every user of an application built out of tree?

$(error Application parent directory is not listed in the list of examples \
or tests subdirectories. Please update 'makefiles/app_dirs.inc.mk')
endif

# Use TOOLCHAIN environment variable to select the toolchain to use.
# If native, TOOLCHAIN for OSX is llvm
ifeq ($(BOARD),native)
Expand Down
51 changes: 45 additions & 6 deletions makefiles/app_dirs.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,55 @@
# fallback so empty RIOTBASE won't lead to "/examples/"
RIOTBASE ?= .

# Define the list of examples subdirectories that contain application directories
EXAMPLES_APPLICATIONS_SUBDIRS := \
ble \
coap \
cord \
dtls \
gnrc \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could change with the discussion below, but I'm not sure if adding gnrc here instead of ipv6, udp is a good idea. I expect that most newcomers would try to search for those and gnrc doesn't say too much unless someone uses RIOT beforehand.

For instance, where would we put GNRC LoRaWAN? gnrc? lora?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I just saw #15358 (comment).

But in any case, I still think it would be better to have a more "functional" name than gnrc here. All the examples mentioned in #15358 (comment) have different categories (gnrc_border_router is a border router, gnrc_networking_mac is just using one MAC layer, etc).

I think it would make sense something like:

- border_routers
    - gnrc_border_router
    - openthread_ncp (coming soon)
- ieee802154
    - lwmac
    - gomac
    - submac
-  netif
    - gnrc_networking

or something like that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keep in mind that one day some applications in the tests might arrive in the examples directory. The current proposal is a way to organize the "storage" of applications in the repo. How well it's organized is not so important IMHO and it will be difficult to find an organization that fits with the diversity of applications. In the long term, what would be beneficial is to add another layer of documentation (using Sphinx, who knows) where example applications could be referenced in different categories (networking, lorawan, gnrc => gnrc_lorawan, etc) but the application would remain stored in the examples/lorawan, because it's the most obvious category.

I fear that it will be impossible to find an organization that fits everyone vision and that this discussion will be endless.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I see your point and agree. I also agree that this wouldn't replace documentation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fear that it will be impossible to find an organization that fits everyone vision and that this discussion will be endless.

I agree here, a first step is having an organization, moving that organization around should be easier once the initial change is in.

icn \
lorawan \
mqtt \
posix \
#
EXAMPLES_APPLICATIONS_SUBDIRS := $(addprefix examples/,$(EXAMPLES_APPLICATIONS_SUBDIRS))

# Define the list of tests sudirectories that contain application directories
TEST_APPLICATIONS_SUBDIRS := \
arch \
bench \
boards \
bootloaders \
build_system \
core \
cpp \
drivers \
periph \
pkg \
sys \
sys/net \
#
TEST_APPLICATIONS_SUBDIRS := $(addprefix tests/,$(TEST_APPLICATIONS_SUBDIRS))

# Prepare the list of application directories
APPLICATION_DIRS := \
fuzzing \
bootloaders \
examples \
tests \
$(EXAMPLES_APPLICATIONS_SUBDIRS) \
$(TEST_APPLICATIONS_SUBDIRS) \
#

# 1. use wildcard to find Makefiles
# 2. use patsubst to drop trailing "/"
# 3. use patsubst to drop possible leading "./"
# 4. sort
APPLICATION_DIRS := $(sort $(patsubst ./%,%,$(patsubst %/,%,$(dir $(wildcard \
$(RIOTBASE)/fuzzing/*/Makefile \
$(RIOTBASE)/bootloaders/*/Makefile \
$(RIOTBASE)/examples/*/Makefile \
$(RIOTBASE)/tests/*/Makefile \
)))))
APPLICATION_DIRS := $(addprefix $(RIOTBASE)/,$(APPLICATION_DIRS))
APPLICATION_DIRS_RELATIVE := $(dir $(wildcard $(addsuffix /*/Makefile,$(APPLICATION_DIRS))))
APPLICATION_DIRS_ABSOLUTE := $(abspath $(APPLICATION_DIRS_RELATIVE))
APPLICATION_DIRS := $(sort $(patsubst ./%,%,$(patsubst %/,%,$(APPLICATION_DIRS_RELATIVE))))

info-applications:
@for dir in $(APPLICATION_DIRS); do echo $$dir; done
Expand Down