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

Makefile.base: use thin static archives. #10195

Merged
merged 3 commits into from
Sep 15, 2019
Merged
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
4 changes: 3 additions & 1 deletion Makefile.base
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,12 @@ $(BINDIR)/$(MODULE)/:

$(BINDIR)/$(MODULE).a $(OBJ): | $(BINDIR)/$(MODULE)/

relpath = $(shell realpath --relative-to=$(abspath .) $(1))

$(BINDIR)/$(MODULE).a: $(OBJ) | $(DIRS:%=ALL--%)
@# Recreate archive to cleanup deleted/non selected source files objects
$(Q)$(RM) $@
$(Q)$(AR) $(ARFLAGS) $@ $^
$(Q)$(AR) $(ARFLAGS) $(foreach f,$@ $^,$(call relpath,$f))

CXXFLAGS = $(filter-out $(CXXUWFLAGS), $(CFLAGS)) $(CXXEXFLAGS)
CCASFLAGS = $(filter-out $(CCASUWFLAGS), $(CFLAGS)) $(CCASEXFLAGS)
Expand Down
43 changes: 43 additions & 0 deletions Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,49 @@ print-size: $(ELFFILE)

endif # BUILD_IN_DOCKER

# Rules to check the correctness of thin archives.

relpath = $(shell realpath --relative-to=$(abspath .) $(1))

# Each ARCHECK file contains all the absolute paths found inside the archive.
BASELIB_ARCHECKS = $(patsubst %.a,%.a-check,$(filter %.a,$(BASELIBS)))

# For each a file, print the absolute paths found inside it
# If "ar t" is called with an absolute path it will print an abs path regardless
# of how the archive is internally
# In case of a malformed archive, ar prints to stderr (and sets an error code).
# Doing `2>&1` is hacky, the correct thing would be to get the exit code.
# The `| %.a` is necessary to be able to check file produced in docker.
%.a-check: %.a
$(Q)$(AR) t $(call relpath,$<) 2>&1 | grep '^/' | '$(LAZYSPONGE)' $(LAZYSPONGE_FLAGS) '$@'

# There's no point on keeping files whose content is later copied to another file
.INTERMEDIATE: $(BASELIB_ARCHECKS)

ARCHIVE_CHECK = $(BINDIR)/$(APPLICATION).archive-check

$(ARCHIVE_CHECK): $(BASELIB_ARCHECKS)
$(Q)cat $^ | '$(LAZYSPONGE)' $(LAZYSPONGE_FLAGS) '$@'

# Rule to check if thin archives are correctly produced, that is, with a correct
# relative path.
ifeq ($(BUILD_IN_DOCKER),1)
Copy link
Contributor

Choose a reason for hiding this comment

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

The archive-check does not need to be done in docker I think.

Using relative path for archives was done to ensure that a file generated in docker are still valid in the host system.
So checking on the host system when build in docker should be enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I disable docker support for this one it means that either:

  1. The user must have the toolchain available locally (to use $(AR))
  2. The native ar could be used, but then another variable should be created to hold it's name (someting like $(NATIVE_AR))

I don't think this is a big deal since it is just a diagnostic tool.

archive-check: ..in-docker-container
else
archive-check: $(ARCHIVE_CHECK) FORCE
@if [ -s '$<' ] ; then \
$(COLOR_ECHO) '$(COLOR_RED)Found the following absolute paths in archives' ;\
cat '$<';\
$(COLOR_ECHO) '$(COLOR_RESET)' ;\
exit 1;\
elif [ -f '$<' ] ; then \
$(COLOR_ECHO) '$(COLOR_GREEN)Archives correctly formed$(COLOR_RESET)' ;\
else \
$(COLOR_ECHO) '$(COLOR_RED)Unexpected error (file not found)$(COLOR_RESET)' ;\
exit 1;\
fi
jcarrano marked this conversation as resolved.
Show resolved Hide resolved
endif # BUILD_IN_DOCKER

# Check given command is available in the path
# check_cmd 'command' 'description'
define check_cmd
Expand Down
2 changes: 1 addition & 1 deletion makefiles/cflags.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,5 @@ CFLAGS += $(filter-out $(OPTIONAL_CFLAGS_BLACKLIST),$(OPTIONAL_CFLAGS))
# Default ARFLAGS for platforms which do not specify it.
# Note: make by default provides ARFLAGS=rv which we want to override
ifeq ($(origin ARFLAGS),default)
ARFLAGS = rcs
ARFLAGS = rcTs
endif
1 change: 1 addition & 0 deletions makefiles/docker.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export DOCKER_MAKECMDGOALS_POSSIBLE = \
scan-build \
scan-build-analyze \
tests-% \
archive-check \
#
export DOCKER_MAKECMDGOALS = $(filter $(DOCKER_MAKECMDGOALS_POSSIBLE),$(MAKECMDGOALS))

Expand Down
2 changes: 1 addition & 1 deletion sys/arduino/Makefile.include
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

# Define application sketches module, it will be generated into $(BINDIR)
SKETCH_MODULE ?= arduino_sketches
SKETCH_MODULE_DIR ?= $(BINDIR)/$(SKETCH_MODULE)
SKETCH_MODULE_DIR ?= $(BINDIR)/$(SKETCH_MODULE)_src
SKETCHES = $(wildcard $(APPDIR)/*.sketch)
include $(RIOTBASE)/sys/arduino/sketches.inc.mk

Expand Down
3 changes: 2 additions & 1 deletion sys/arduino/sketches.inc.mk
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ SKETCH_GENERATED_FILES = $(SKETCH_MODULE_DIR)/Makefile $(SKETCH_MODULE_DIR)/$(SK
# Building the module files
# Do not use $^ in receipes as Makefile is also a prerequisite
$(SKETCH_MODULE_DIR)/Makefile: $(SKETCH_MODULE_DIR)/$(SKETCH_CPP)
$(Q)echo 'SRCXX = $(SKETCH_CPP)' > $@
$(Q)echo 'MODULE = $(SKETCH_MODULE)' > $@
$(Q)echo 'SRCXX = $(SKETCH_CPP)' >> $@
$(Q)echo 'include $$(RIOTBASE)/Makefile.base' >> $@
$(SKETCH_MODULE_DIR)/$(SKETCH_CPP): $(SKETCHES_ALL)
@mkdir -p $(@D)
Expand Down