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

CB-13964: Fixed make clean for windows #806

Merged
merged 4 commits into from Mar 19, 2018
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 21 additions & 2 deletions Makefile
Expand Up @@ -50,6 +50,11 @@ else
printfile = cat $(1)
endif

ifdef WINDOWS
RM = cmd /C del /Q /F $(subst /,\,$(1))
RMDIR = cmd /C rmdir /S /Q $(subst /,\,$(1))
endif
Copy link
Contributor

Choose a reason for hiding this comment

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

You can make non-Windows versions of these, and then you won't have to use Windows-specific logic in the clean target. So, add an else clause and just define:

else
RM = rm -f $(1)
RMDIR = rm -rf $(1)
endif


# constants
EMPTY =
SPACE = $(EMPTY) $(EMPTY)
Expand Down Expand Up @@ -152,7 +157,7 @@ NEXT_DOCS_TOCS = $(addprefix $(TOC_DIR)/,$(addsuffix _$(NEXT_DOCS_VERSION_SLUG)-
JEKYLL_CONFIGS = $(MAIN_CONFIG) $(DEFAULTS_CONFIG) $(VERSION_CONFIG)
JEKYLL_FLAGS =

BUILD_DATA = $(DOCS_VERSION_DATA) $(DOCS_PAGE_LIST) $(TOC_FILES)
BUILD_DATA = $(TOC_FILES) $(DOCS_VERSION_DATA) $(DOCS_PAGE_LIST)

# convenience targets
help usage default:
Expand Down Expand Up @@ -287,6 +292,19 @@ $(CSS_DEST_DIR)/%.css: $(CSS_SRC_DIR)/%.css
$(call printfile,$<) >> $@

# maintenance
ifdef WINDOWS
clean:
$(call RM, $(VERSION_CONFIG))
$(call RM, $(DEFAULTS_CONFIG))
$(call RM, $(DOCS_PAGE_LIST))
$(call RM, $(DOCS_VERSION_DATA))
$(call RM, $(TOC_FILES))
$(call RM, $(PLUGINS_APP))
$(call RM, $(FETCHED_FILES))
-$(call RMDIR, $(CSS_DEST_DIR))
-$(call RMDIR, $(PROD_DIR))
-$(call RMDIR, $(DEV_DIR))
else
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have RM and RMDIR defined for all platforms, you don't have to add Windows-specific logic here.

clean:
$(RM) $(VERSION_CONFIG)
$(RM) $(DEFAULTS_CONFIG)
Expand All @@ -297,9 +315,10 @@ clean:
$(RM) $(PLUGINS_APP)
$(RM) -r $(CSS_DEST_DIR)
$(RM) $(FETCHED_FILES)
endif

nuke: clean
$(RM) -r node_modules
$(RM) Gemfile.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

These also need to be changed to work with the new RM and RMDIR.


.PHONY: clean usage help default build fetch $(DEV_DOCS)
.PHONY: clean usage help default build fetch $(DEV_DOCS)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: no newline at end of file.