Supplying per-file compiler flags

[Sipkab]

Missing feature
It is currently very hackish, to add per-file compiler flags using the Android.mk makefile.
Consider the following scenario:

LOCAL_CPPFLAGS := -DGLOBAL_DEFINE
LOCAL_SRC_FILES += file1.cpp
LOCAL_SRC_FILES += file2.cpp

#Here I would like to add a compiler flag, only for the file1.cpp, for example -DFILE1_DEFINE

Hackish solution
There's no public API for doing this, and it could be useful in the future releases.
At the moment, it could be done like this, but upcoming versions can break it:
(the solution was created by examining definitions.mk, and creating these targets in Android.mk)

objs/local/armeabi-v7a/objs-debug/main/file1.o: override PRIVATE_CXX := $(NDK_CCACHE) $($(my)CXX) -DFILE1_DEFINE
objs/local/armeabi-v7a/objs-debug/main/file1.o: override PRIVATE_CC := $(NDK_CCACHE) $($(my)CXX) -DFILE1_DEFINE

Proposed Solution
This problem could be solved, if the build targets would depend on some phony targets, like the following:

#this is in the private makefiles (such as definitions.mk)
objs/local/armeabi-v7a/objs-debug/main/file1.o: \
FLAGS_file1.o \
FLAGS_armeabi-v7a_file1.o \
#and more with the remaining supported architectures.

And in the Android.mk:

FLAGS_file1.o: FILE_FLAGS := -DFILE1_DEFINE

This way the developer could supply different flags for different architectures, and-or common flags for all the architectures.
(The targets have the format FLAGS_arch_path/to/object/file.o)

[tavianator]

Does

objs/local/%/objs-debug/main/file1.o: LOCAL_CPPFLAGS += -DFILE1_DEFINE

work?

[Sipkab]

No. I have the following setup:
file1.cpp

#ifndef FILE1_DEFINE
#error test fail
#endif

file2.cpp

#ifdef FILE1_DEFINE
#error test fail
#endif

I tried your example, but it still fails, because the define is missing. Even when I specify armeabi-v7a instead of %.
When I print out the current value of the LOCAL_CPPFLAGS when the dependency is resolved, it prints out empty.

CPPFLAGS:
[armeabi-v7a] Compile++ thumb: main <= file1.cpp

The method of printing:

.PHONY: printer
printer:
    @echo CPPFLAGS: $(LOCAL_CPPFLAGS)

objs/local/armeabi-v7a/objs-debug/main/file1.o: printer

(When I try to use objs/local/%/objs-debug/main/file1.o as my printing dependency, then it doesn't print it out at all.)

[DanAlbert]

The way to do this in ndk-build is to put files that need different cflags in separate (static library) modules and link them into the final shared library using LOCAL_WHOLE_STATIC_LIRBRAIRES.

Note that you can do what you described with cmake: http://stackoverflow.com/a/13639476/632035

[DanAlbert]

Rather than "wontfix", let's keep this open in unplanned. I don't think any of us will get to this any time soon, but if cmake fails to take over then we may want to reconsider that.

[Sipkab]

So as you labeled the issue with "help wanted" I decided to make some progress for it.
I successfully managed to implement this, only modifying definitions.mk a little bit in the following way:

I added a new function, that makes a request to the developer to specify the "per-file-flags" for the current source:

# Requests the developer to modify the PER_FILE_FLAGS_$@ (meaning PER_FILE_FLAGS_<target name>) 
# variable in order to specify flags for individual files
# This assumes the following variables defined
# _OBJ: destination file
# Arguments : 1: the name of the source file
# Usage: on build command: $$(PER_FILE_FLAGS_$$(PRIVATE_PER_FILE_FLAGS_POSTFIX))
#        by developer:
#            create FLAGS_<path to source> target, and evaluate $(eval PER_FILE_FLAGS_$@ := <additional flags>)
# Outputs: PRIVATE_PER_FILE_FLAGS_POSTFIX variable, containing the postfix for the unique flag variable 
#              the postfix equals to the following: FLAGS_<argument 1>
#          PER_FILE_FLAGS_<postfix> variable, containing the additional flags, or empty
define ev-get-per-file-flags
.PHONY: FLAGS_$$(1)
$$(_OBJ): FLAGS_$$(1)
$$(_OBJ): PRIVATE_PER_FILE_FLAGS_POSTFIX := FLAGS_$$(1)
endef

This creates a new dependency on a target named FLAGS_<path-to-source>, where the user can specify his flags like this:

FLAGS_path/to/my/file.cpp: 
    $(eval PER_FILE_FLAGS_$@ := -DFILE1_DEFINE)

The PER_FILE_FLAGS_$@ ensures that an unique global variable is created, so when -j is specified on the command line, no concurrency issues will occur.

The function sould be called in definitions.mk, before the build command is executed.
$$(eval $$(call ev-get-per-file-flags, $$(PRIVATE_SRC))) (Or PRIVATE_CPP_SRCin one place)

This solution adds the following command line argument to the build command:

$$(PER_FILE_FLAGS_$$(PRIVATE_PER_FILE_FLAGS_POSTFIX))

This is fixed in definitions.mk in 4 places.
I upload the new definitions.mk file, which is based on the ndk version r12b.
definitions.mk.zip
(Github doesn't support mk, so it's a zip)
I'm not a makefile-pro, but I hope my solution will stand.
Hope to see this in the upcoming release. :)

[DanAlbert]

First, thanks for looking in to this :)

I don't think this is how we want to present this option to the user. It doesn't match with the rest of the Android.mk language. Instead of

FLAGS_path/to/my/file.cpp: 
    $(eval PER_FILE_FLAGS_$@ := -DFILE1_DEFINE)

I was thinking this would take the form of

LOCAL_CFLAGS_filename.cpp :+= -Dfoo

One thing to remember here is that it will need to support LOCAL_CFLAGS, LOCAL_CPPFLAGS, LOCAL_CONLYFLAGS, LOCAL_ASFLAGS, and LOCAL_ASMFLAGS (I don't think I missed any).

This is also going to require a test. You'll need to check out the NDK to run the tests: https://android.googlesource.com/platform/ndk/ (the README.md there should get you started, also note the link at the top pointing at our testing docs). Only a build test is needed here; a device test wouldn't add any additional value.

You should be able to clone just that project to run the tests against your locally modified NDK (i.e. you shouldn't need to do the repo init steps or even build the NDK). Something like python tests/run-all.py --filter your-test --abi armeabi-v7a path/to/your/ndk.

[Sipkab]

Thanks, I'll have a look at it soon.
Btw, what do you mean by LOCAL_ASFLAGS? I've not found any trace of it in my version of the ndk. I've found however one you didn't mention: LOCAL_RENDERSCRIPT_FLAGS.

[DanAlbert]

Whoops, thought I replied to this.

LOCAL_ASFLAGS wasn't in r12, but was added in master (and is in the recently released r13 beta 1).

[alexcohn]

Some while ago, I published a hack that seems to work until r16. This, for example, will define t_DEFINE and qq_DEFINE, respectively:

get-src-file-target-cflags = $(LOCAL_SRC_FILES_TARGET_CFLAGS.$1) -D$(basename $1)_DEFINE

LOCAL_PATH := $(call my-dir)

include $(CLEAR_VARS)
LOCAL_SRC_FILES := t.cpp qq.c
LOCAL_MODULE := tm
LOCAL_LDLIBS := -latomic

include $(BUILD_SHARED_LIBRARY)

[emileb]

Taking inspiration from @Sipkab above I edited the 'ndk-bundle\build\core\definitions.mk' file in the NDK as follows:

Find the following code and add this line:

$($(subst /,_,$(1))_CFLAGS) \

Like so:

define  ev-compile-c-source 
_SRC:=$$(call local-source-file-path,$(1)) 
_OBJ:=$$(LOCAL_OBJS_DIR:%/=%)/$(2)
_FLAGS := $$($$(my)CFLAGS) \
          $$(call get-src-file-target-cflags,$(1)) \
          $$(call host-c-includes,$$(LOCAL_C_INCLUDES) $$(LOCAL_PATH)) \
          $$(NDK_APP_CFLAGS) \
          $$(NDK_APP_CONLYFLAGS) \
          $$(LOCAL_CFLAGS) \
          $$(LOCAL_CONLYFLAGS) \
          $($(subst /,_,$(1))_CFLAGS) \               <<----- NEW LINE
          --sysroot $$(call host-path,$$(SYSROOT_INC)) \
          $(SYSROOT_ARCH_INC_ARG) \
          -c \

_TEXT := Compile $$(call get-src-file-text,$1)
_CC   := $$(NDK_CCACHE) $$(TARGET_CC)

$$(eval $$(call ev-build-source-file))
endef

Do the same for the CPP (define ev-compile-cpp-source) a bit further down.

Now in your Android.mk file you can add extra flags like so:

(source name)_CFLAGS := -DMY_DEFINE

e.g:

main.cpp_CFLAGS := -O1

Forward slash path separators are turned into underscores:

mypath_main.cpp_CFLAGS := -DHELLO -O2

[mm-longcheng]

https://stackoverflow.com/questions/7679363/android-build-system-neon-and-non-neon-builds
LOCAL_NEON_SRC_FILES := imgproc/neon_utils.c
LOCAL_NEON_CFLAGS := -mfloat-abi=softfp -mfpu=neon -march=armv7
TARGET-process-src-files-tags += $(call add-src-files-target-cflags, $(LOCAL_NEON_SRC_FILES), $(LOCAL_NEON_CFLAGS))

[alexcohn]

@mm-longcheng: it seems that this old trick of mine still works in r17!

[noloader]

This is probably going to sound awful, but the work-arounds are awful. None of them scale the way we need them to when converting/porting a makefile like GNUmakefile.

In our regular makefile, we have recipes like the following. CRC_FLAG gets a feature/compile test, so we set CRC_FLAG to one of -msse4.2, -march=armv8-a or -DDISABLE_CRC=1 (with some hand waiving):

# SSE4.2 or ARMv8a available
crc_simd.o : crc_simd.cpp
    $(CXX) $(strip $(CPPFLAGS) $(CXXFLAGS) $(CRC_FLAG) -c) $<

Notice we don't have to build separate static libraries or jump through other hoops. (I do have to use the static library trick in Autotools. It does not scale.)

Since the user owns CFLAGS and CXXFLAGS, perhaps Android can take ownership of AOSP_CFLAGS and AOSP_CXXFLAGS and add all of its regular flags. Then we add what we need and supply the recipe:

# SSE4.2 or ARMv8a available
crc_simd.o : crc_simd.cpp
    $(CXX) $(strip $(CPPFLAGS) $(AOSP_CXXFLAGS) $(CXXFLAGS) $(CRC_FLAG) -c) $<

If Android supplies its flags in AOSP_CXXFLAGS and the recipe gets copied verbatim, then we should be able to build to our specification without a lot grief.

Or maybe better, allow us to drop this in our Android.mk, and have the backend work the magic to automatically insert AOSP_CXXFLAGS after the compiler and before the user's CXXFFLAGS. This would be the best solution for me and other users since it requires the least amount of learning and work.

# SSE4.2 or ARMv8a available
crc_simd.o : crc_simd.cpp
    $(CXX) $(strip $(CPPFLAGS) $(CXXFLAGS) $(CRC_FLAG) -c) $<

[alexcohn]

@noloader what's wrong with the following? This can be generated from your GNUmakefile easily:

get-src-file-target-cflags = $(LOCAL_SRC_FILES_TARGET_CFLAGS.$1) $($(basename $1)_FILE_FLAGS)

aes_armv4_FILE_FLAGS := $(CRYPTOGAMS_AES_FLAG)

# SSSE3 or NEON available
aria_simd_FILE_FLAGS := $(ARIA_FLAG)

# SSE, NEON or POWER7 available
blake2s_simd_FILE_FLAGS := $(BLAKE2S_FLAG)

# SSE, NEON or POWER8 available
blake2b_simd_FILE_FLAGS := $(BLAKE2B_FLAG)

# SSE2 or NEON available
chacha_simd_FILE_FLAGS := $(CHACHA_FLAGi)

LOCAL_PATH := $(call my-dir)

include $(CLEAR_VARS)
LOCAL_SRC_FILES := $(DLLSRCS)
LOCAL_MODULE := cryptopp

include $(BUILD_SHARED_LIBRARY)
```~                              
    

[DanAlbert]

I doubt this is something we'll ever do. There doesn't seem to be much demand for it, and understandably so (if you need it, that's probably a symptom of a different problem or a different missing feature). If something changes that makes this worth adding we can reopen, but for now I'm closing.