CB-13964: Fixed make clean for windows #806
Conversation
Makefile
Outdated
| @@ -14,6 +14,8 @@ SHELL = cmd | |||
| JEKYLL = bundle.bat exec jekyll | |||
| CAT = type | |||
| LS = ls | |||
| RM = cmd /C del /Q /F | |||
| RMDIR = cmd /C rmdir /S /Q | |||
There was a problem hiding this comment.
Nitpick: align equals signs.
Makefile
Outdated
| PLUGINS_APP := $(subst /,\,$(PLUGINS_APP)) | ||
| CSS_DEST_DIR := $(subst /,\,$(CSS_DEST_DIR)) | ||
| FETCHED_FILES := $(subst /,\,$(FETCHED_FILES)) | ||
| endif |
There was a problem hiding this comment.
This could break other uses of these files. Instead, change the definition of RM and RMDIR to escape slashes, like how makedir does it here.
Makefile
Outdated
| $(RM) $(FETCHED_FILES) | ||
| IF EXIST "$(CSS_DEST_DIR)" $(RMDIR) $(CSS_DEST_DIR) | ||
| IF EXIST "$(PROD_DIR)" $(RMDIR) $(PROD_DIR) | ||
| IF EXIST "$(DEV_DIR)" $(RMDIR) $(DEV_DIR) |
There was a problem hiding this comment.
As above, you can avoid the if/else here by defining RM differently. Also, you can avoid the IF EXIST by adding a - to the beginning of the line, which will tell Make to ignore any errors. So:
-$(RMDIR) $(CSS_DEST_DIR)
instead of
$(RMDIR) $(CSS_DEST_DIR)
Makefile
Outdated
|
|
||
| nuke: clean | ||
| $(RM) -r node_modules | ||
| $(RM) Gemfile.lock | ||
|
|
||
| .PHONY: clean usage help default build fetch $(DEV_DOCS) | ||
| .PHONY: clean usage help default build fetch $(DEV_DOCS) |
There was a problem hiding this comment.
Nitpick: no newline at end of file.
Makefile
Outdated
| ifdef WINDOWS | ||
| RM = cmd /C del /Q /F $(subst /,\,$(1)) | ||
| RMDIR = cmd /C rmdir /S /Q $(subst /,\,$(1)) | ||
| endif |
There was a problem hiding this comment.
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
Makefile
Outdated
| -$(call RMDIR, $(CSS_DEST_DIR)) | ||
| -$(call RMDIR, $(PROD_DIR)) | ||
| -$(call RMDIR, $(DEV_DIR)) | ||
| else |
There was a problem hiding this comment.
If you have RM and RMDIR defined for all platforms, you don't have to add Windows-specific logic here.
Makefile
Outdated
| $(call RM, $(FETCHED_FILES)) | ||
| -$(call RMDIR, $(CSS_DEST_DIR)) | ||
| -$(call RMDIR, $(PROD_DIR)) | ||
| -$(call RMDIR, $(DEV_DIR)) | ||
|
|
||
| nuke: clean | ||
| $(RM) -r node_modules | ||
| $(RM) Gemfile.lock |
There was a problem hiding this comment.
These also need to be changed to work with the new RM and RMDIR.
|
@gandhirajan these changes look good to me |
This PR fixes 'make clean' command issue in windows OS.