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.

Sign up for GitHub

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

Jump to bottom

Fix the make all-keyboards command #422

Merged
merged 1 commit into from
Jun 21, 2016
Merged

Fix the make all-keyboards command #422

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
35 changes: 26 additions & 9 deletions tmk_core/rules.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,17 +670,34 @@ all-keyboards-defaults-%:
all-keyboards-defaults: all-keyboards-defaults-all

KEYBOARDS := $(SUBDIRS:$(TOP_DIR)/keyboard/%/=/keyboard/%)
all-keyboards-%: $(KEYBOARDS)
/keyboard/%:
$(eval KEYBOARD=$(patsubst /keyboard/%,%,$@))
$(eval KEYMAPS=$(notdir $(patsubst %/.,%,$(wildcard $(TOP_DIR)$@/keymaps/*/.))))
@for x in $(KEYMAPS) ; do \
printf "Compiling $(BOLD)$(KEYBOARD)$(NO_COLOR) with $(BOLD)$$x$(NO_COLOR)" | $(AWK) '{ printf "%-88s", $$0; }'; \
LOG=$$($(MAKE) -C $(TOP_DIR)$@ $(subst all-keyboards-,,$@) keymap=$$x VERBOSE=$(VERBOSE) COLOR=$(COLOR) SILENT=true 2>&1) ; if [ $$? -gt 0 ]; then $(PRINT_ERROR_PLAIN); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING_PLAIN); else $(PRINT_OK); fi; \
done

all-keyboards-all: $(addsuffix -all,$(KEYBOARDS))
all-keyboards-quick: $(addsuffix -quick,$(KEYBOARDS))
all-keyboards-clean: $(addsuffix -clean,$(KEYBOARDS))
Copy link
Contributor

Choose a reason for hiding this comment

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

If I remember my make-fu right, this should work:

all-keyboards-%: $(addsuffix -$*,${KEYBOARDS})

Copy link
Contributor Author

@fredizzimo fredizzimo Jun 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@algernon

That is what I thought too, and I tried many variations of this yesterday, and now again, this is what I get

$ make all-keyboards-quick
make: *** No rule to make target 'all-keyboards-quick'.  Stop.

If I do this instead though

all-keyboards-%:
    echo $(addsuffix -$*,${KEYBOARDS})

It prints out the correct rule names, so the prerequisites pattern is right. I also tried adding the debug flag to make, and it didn't help me debug the problem either. But maybe someone with better makefile programming skills could?

all-keyboards: all-keyboards-all

define make_keyboard
$(eval KEYBOARD=$(patsubst /keyboard/%,%,$1))
$(eval KEYMAPS=$(notdir $(patsubst %/.,%,$(wildcard $(TOP_DIR)$1/keymaps/*/.))))
@for x in $(KEYMAPS) ; do \
printf "Compiling $(BOLD)$(KEYBOARD)$(NO_COLOR) with $(BOLD)$$x$(NO_COLOR)" | $(AWK) '{ printf "%-88s", $$0; }'; \
LOG=$$($(MAKE) -C $(TOP_DIR)$1 $2 keymap=$$x VERBOSE=$(VERBOSE) COLOR=$(COLOR) SILENT=true 2>&1) ; if [ $$? -gt 0 ]; then $(PRINT_ERROR_PLAIN); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING_PLAIN); else $(PRINT_OK); fi; \
done
endef

define make_keyboard_helper
# Just remove the -quick, -all and so on from the first argument and pass it forward
$(call make_keyboard,$(subst -$2,,$1),$2)
endef

/keyboard/%-quick:
$(call make_keyboard_helper,$@,quick)
/keyboard/%-all:
$(call make_keyboard_helper,$@,all)
/keyboard/%-clean:
$(call make_keyboard_helper,$@,clean)
/keyboard/%:
$(call make_keyboard_helper,$@,all)

Copy link
Contributor

@algernon algernon Jun 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

/keyboard/%: /keyboard/%-%:
    $(call make_keyboard_helper,$@,$*)

This may be able to replace all the /keyboard/%-blah rules. But I haven't tested it, and my makefile knowledge is a bit rusty.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@algernon

Unfortunately that doesn't work either,

Replacing the rules with this

/keyboard/%: /keyboard/%-%:
    $(call make_keyboard_helper,$@,$*)

gives

$ make all-keyboards-quick
tmk_core/rules.mk:692: *** mixed implicit and static pattern rules.  Stop.

One idea that I had was to use the subst function to replace all dashes with spaces, and then use the lastword function to get quick, all, clean and so on. But I decided against that, since it can easily start causing troubles if we add keyboards with dashes in the name.

all-keymaps-%:
$(eval MAKECONFIG=$(call get_target,all-keymaps,$@))
$(eval KEYMAPS=$(notdir $(patsubst %/.,%,$(wildcard $(TOP_DIR)/keyboard/$(KEYBOARD)/keymaps/*/.))))
Expand Down