Arduino-Makefile icon indicating copy to clipboard operation
Arduino-Makefile copied to clipboard

feat(options): add SRC_DIR var to allow sources to live in a subfolder

Open mgcrea opened this issue 9 years ago • 12 comments

Would fix #267.

mgcrea avatar Jan 30 '16 21:01 mgcrea

+1 for this, having a Makefile in the project's root and the project's source in the src dir is a pretty common thing for C projects.

@sudar What's needed for this to be merged? And can you trigger Travis again? Seems like it got stuck on something unrelated.

simonvanderveldt avatar Jun 12 '16 15:06 simonvanderveldt

@sej7278 What are your thoughts on this?

@simonvanderveldt

I don't see a way to restart the Travis test again. Let me see if I can manually trigger it somehow.

sudar avatar Jun 13 '16 04:06 sudar

i know the ide is going to support searching the src/ subdirectory in 1.6.10 instead of recursively scanning all subdirectories that it currently does in 1.6.9 - which breaks cached rebuilds and you have to make clean every time! see https://github.com/arduino/arduino-builder/issues/89

could we append rather than overwrite the variables, so we can use the current directory as well as the src/ subdirectory? as it is if SRC_DIR is defined you have to use it exclusively.

i'd rather we followed the ide and hardcoded SRC_DIR as "src/", like libraries do or at least initially define it as src/ e.g. $SRC_DIR ?= src

-LOCAL_C_SRCS    ?= $(wildcard *.c)
-LOCAL_CPP_SRCS  ?= $(wildcard *.cpp)
-LOCAL_CC_SRCS   ?= $(wildcard *.cc)
-LOCAL_PDE_SRCS  ?= $(wildcard *.pde)
-LOCAL_INO_SRCS  ?= $(wildcard *.ino)
-LOCAL_AS_SRCS   ?= $(wildcard *.S)
+SRC_DIR         ?= src
+LOCAL_C_SRCS    ?= $(wildcard $(SRC_DIR)/*.c *.c)
+LOCAL_CPP_SRCS  ?= $(wildcard $(SRC_DIR)/*.cpp *.cpp)
+LOCAL_CC_SRCS   ?= $(wildcard $(SRC_DIR)/*.cc *.cc)
+LOCAL_PDE_SRCS  ?= $(wildcard $(SRC_DIR)/*.pde *.pde)
+LOCAL_INO_SRCS  ?= $(wildcard $(SRC_DIR)/*.ino *.ino)
+LOCAL_AS_SRCS   ?= $(wildcard $(SRC_DIR)/*.S *.S)

by using USER_LIB_PATH we can also have libraries in the subdirectory too, this works:

BOARD_TAG = leonardo
MONITOR_PORT = /dev/ttyACM0
ARDUINO_LIBS = WiiChuck Wire Servo
USER_LIB_PATH = src/libs

include $(HOME)/Arduino-Makefile/Arduino.mk
├── Makefile
└── src
    ├── libs
    │   └── WiiChuck
    ├── pindefs.h
    └── wiichuck_buttons.ino

need to add some info to HISTORY.md and arduino-mk-vars.md

pip keeps breaking travis for some reason, temp files not being cleared or something.

sej7278 avatar Jun 13 '16 09:06 sej7278

could we append rather than overwrite the variables, so we can use the current directory as well as the src/ subdirectory? as it is if SRC_DIR is defined you have to use it exclusively.

But isn't only using SRC_DIR for your sources the thing you'd want to achieve when you set it? At least for me it is, the "magic" of also including other source dirs than the one I explicitly set as a user would just make unintentional files being included a bigger possibility.

simonvanderveldt avatar Jun 14 '16 07:06 simonvanderveldt

yes its a good point perhaps its better to explicitly set the SRC_DIR (or not) rather than also include files from the CWD that may confuse things. you'd have to document that you need a trailing /

sej7278 avatar Jun 14 '16 08:06 sej7278

we should probably output SRC_DIR if its set to the Arduino.mk configuration screen like:

Arduino.mk Configuration:
...
- [USER]               SRC_DIR = src/ 
diff --git a/Arduino.mk b/Arduino.mk
index 322ca38..7957a34 100644
--- a/Arduino.mk
+++ b/Arduino.mk
@@ -762,12 +762,12 @@ endif
 ########################################################################
 # Local sources

-LOCAL_C_SRCS    ?= $(wildcard *.c)
-LOCAL_CPP_SRCS  ?= $(wildcard *.cpp)
-LOCAL_CC_SRCS   ?= $(wildcard *.cc)
-LOCAL_PDE_SRCS  ?= $(wildcard *.pde)
-LOCAL_INO_SRCS  ?= $(wildcard *.ino)
-LOCAL_AS_SRCS   ?= $(wildcard *.S)
+LOCAL_C_SRCS    ?= $(wildcard $(SRC_DIR)*.c)
+LOCAL_CPP_SRCS  ?= $(wildcard $(SRC_DIR)*.cpp)
+LOCAL_CC_SRCS   ?= $(wildcard $(SRC_DIR)*.cc)
+LOCAL_PDE_SRCS  ?= $(wildcard $(SRC_DIR)*.pde)
+LOCAL_INO_SRCS  ?= $(wildcard $(SRC_DIR)*.ino)
+LOCAL_AS_SRCS   ?= $(wildcard $(SRC_DIR)*.S)
 LOCAL_SRCS      = $(LOCAL_C_SRCS)   $(LOCAL_CPP_SRCS) \
                $(LOCAL_CC_SRCS)   $(LOCAL_PDE_SRCS) \
                $(LOCAL_INO_SRCS) $(LOCAL_AS_SRCS)
@@ -780,6 +780,10 @@ ifeq ($(words $(LOCAL_SRCS)), 0)
     $(error At least one source file (*.ino, *.pde, *.cpp, *c, *cc, *.S) is needed)
 endif

+ifdef SRC_DIR
+    $(call show_config_variable,SRC_DIR,[USER])
+endif
+
 # CHK_SOURCES is used by flymake
 # flymake creates a tmp file in the same directory as the file under edition
 # we must skip the verification in this particular case

Might be worth merging #429 into this commit too, as its the same area of code....?

sej7278 avatar Jun 14 '16 09:06 sej7278

we should probably output SRC_DIR if its set to the Arduino.mk configuration screen like:

👍

Might be worth merging #429 into this commit too, as its the same area of code....?

Even though that PR touches parts of the same code there doesn't seem to be a real relation between these 2 issues/PRs and there's value in both of them separate from the other one, so I'd leave them as separate PRs.

simonvanderveldt avatar Jun 14 '16 21:06 simonvanderveldt

i just meant this is going to totally break that PR, so maybe it would be nice to figure out a way around that. otherwise if we can get SRC_DIR documented and HISTORY.md updated, it should be good to merge i guess.

sej7278 avatar Jun 14 '16 22:06 sej7278

i just meant this is going to totally break that PR,

Well, one of them has to rebase anyway ;)

simonvanderveldt avatar Jun 15 '16 07:06 simonvanderveldt

@sej7278

Do you still think this PR makes sense?

If yes, then I can update the Readme and documentation and trigger the Travis CI again.

If not, we can close it.

sudar avatar Sep 30 '18 15:09 sudar

its a nice idea, and it looks like it should merge ok :+1:

sej7278 avatar Sep 30 '18 19:09 sej7278

@mgcrea @simonvanderveldt

Can you please add a note about the changes in the History.md file? We would be happy to merge it once we have the updated History.md file.

sudar avatar Oct 07 '18 05:10 sudar