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

Allow assembly source files with a lower-case .s extension #429

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Allow assembly source files with a lower-case .s extension #429

wants to merge 2 commits into from

Conversation

ryansuchocki
Copy link

I was very confused when I first tried to upload an assembly project using Arduino-mk, and it complained that there were no source files.

GCC (and hence, avr-gcc) allows assembly sources with either a lower-case or upper-case extension. I see no reason why Arduino-mk should require one or the other.

@sej7278
Copy link
Collaborator

sej7278 commented May 21, 2016

no complaints from me, bit surprised this hasn't already been done, assumed it was case-insensitive wildcard i guess.

although isn't that assignment going to stop .S files from working?

@ryansuchocki
Copy link
Author

You're right! I think I made some silly mistakes when I was testing this...

I'll try again :)

@sej7278
Copy link
Collaborator

sej7278 commented May 23, 2016

you're not giving up on this are you @ryansuchocki ?

@ryansuchocki
Copy link
Author

Temporarily... I was trying to comprehend the rest of the makefile, to see if this breaks something else, and I panicked!

@sej7278
Copy link
Collaborator

sej7278 commented Feb 19, 2017

@ryansuchocki is this ready to merge - subject to updating HISTORY.md ?

@sej7278
Copy link
Collaborator

sej7278 commented Jun 12, 2018

@sudar can we merge this (manually fixing history i guess)?

edit: no, can't merge as it doesn't extend to the new sam/due support, still lots of use of .S there

@sudar
Copy link
Owner

sudar commented Jun 22, 2018

@sej7278

no, can't merge as it doesn't extend to the new sam/due support, still lots of use of .S there

Is there a easy way to fix this ourself?

@sej7278
Copy link
Collaborator

sej7278 commented Jun 22, 2018

@sudar actually probably just needs rebasing so it applies to lines 793, 799, 803, as we're only changing LOCAL_AS_SRCS provided by the user, not all instances of .S files (e.g. libraries, cores etc.)

@sej7278
Copy link
Collaborator

sej7278 commented Jun 22, 2018

I tried this, but I've no idea how to use a prebuilt assembly file with the makefile, so can't tell if it works. simply putting a blink.s file (from prior generated_assembly) in the project directory seems to break the build.

diff --git a/Arduino.mk b/Arduino.mk
index 979a039..cef790a 100644
--- a/Arduino.mk
+++ b/Arduino.mk
@@ -790,17 +790,17 @@ 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_AS_SRCS   ?= $(wildcard *.S *.s)
 LOCAL_SRCS      = $(LOCAL_C_SRCS)   $(LOCAL_CPP_SRCS) \
                $(LOCAL_CC_SRCS)   $(LOCAL_PDE_SRCS) \
                $(LOCAL_INO_SRCS) $(LOCAL_AS_SRCS)
 LOCAL_OBJ_FILES = $(LOCAL_C_SRCS:.c=.c.o)   $(LOCAL_CPP_SRCS:.cpp=.cpp.o) \
                $(LOCAL_CC_SRCS:.cc=.cc.o)   $(LOCAL_PDE_SRCS:.pde=.pde.o) \
-               $(LOCAL_INO_SRCS:.ino=.ino.o) $(LOCAL_AS_SRCS:.S=.S.o)
+               $(LOCAL_INO_SRCS:.ino=.ino.o) $(LOCAL_AS_SRCS:.%=.%.o)
 LOCAL_OBJS      = $(patsubst %,$(OBJDIR)/%,$(LOCAL_OBJ_FILES))
 
 ifeq ($(words $(LOCAL_SRCS)), 0)
-    $(error At least one source file (*.ino, *.pde, *.cpp, *c, *cc, *.S) is needed)
+    $(error At least one source file (*.ino, *.pde, *.cpp, *c, *cc, *.S, *.s) is needed)
 endif
 
 # CHK_SOURCES is used by flymake
diff --git a/HISTORY.md b/HISTORY.md
index 10b3e5e..1719e9a 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -14,6 +14,7 @@ I tried to give credit whenever possible. If I have missed anyone, kindly add it
 - Tweak: Move non-standard-related items from CxxFLAGS_STD to CxxFLAGS (issue #523) (https://github.com/sej7278)
 - Tweak: Update Windows usage documentation and allow non-relative paths (issue #519) (https://github.com/tuna-f1sh)
 - Tweak: Support Cygwin Unix Python and Windows installation on Windows to pass correct port binding. (https://github.com/tuna-f1sh)
+- Tweak: Allow source files with lower-case .s extension (https://github.com/ryansuchocki)
 - New: Added -fdiagnostics-color to \*STD flags (https://github.com/sej7278)
 - New: Made -fdiagnostics-color take a variiable DIAGNOSTICS_COLOR_WHEN: never, always, auto. (https://github.com/wingunder)
 - New: Add generation of tags file using ctags, which automatically includes project libs and Arduino core. (https://github.com/tuna-f1sh)

@sudar
Copy link
Owner

sudar commented Sep 30, 2018

@sej7278

What are your thoughts on this PR?

Do you think we should rebase and test the changes as you suggested?

@sej7278
Copy link
Collaborator

sej7278 commented Sep 30, 2018

we need an example at least, as we don't even know if it works....

@sudar
Copy link
Owner

sudar commented Oct 7, 2018

@ryansuchocki

Can you give us some examples of how to use the changes that you are proposing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants