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

Misc/makefile improvements #171

Merged
merged 7 commits into from
Mar 13, 2018
Merged
Show file tree
Hide file tree
Changes from 5 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
7 changes: 6 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
*~

# Local files
/local/

# Build artifacts and dependencies
/lib/
/bin/
/.shards/

# Crystal generated docs
/docs/

# The myst executable
/myst

# DWARF file from compiling on macOS
/myst.dwarf
*.dwarf
139 changes: 122 additions & 17 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,22 +1,127 @@
.PHONY: help clean spec mystspec
# Because env `SHELL` might be
# some extremely weird shell
SHELL = bash

spec: ## Runs all specs
crystal spec
crystal run src/myst_cli.cr -- spec/myst/spec.mt
# Parse makefiles named *.mk in the dir `local`
# for local extra makefile targets if wanted.
# It is smart to set MYSTBUILD to your myst dev-build there
-include local/*.mk

myst-spec: ## Runs just the in-language specs
crystal run src/myst_cli.cr -- spec/myst/spec.mt

build: ## Builds myst into an executable
shards build
# https://www.gnu.org/software/make/manual/html_node/One-Shell.html
.ONESHELL:

check: ## Runs all crystal specs
crystal spec/
# The docs says i should do this
.SUFFIXES: .cr .mt
Copy link
Member

Choose a reason for hiding this comment

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

Take a look at https://stackoverflow.com/a/2011058 for why this is useful. A comment like "ensure that only .cr and .mt files can have suffix rules." would be a bit better, but I honestly don't think it's needed here (we're not converting files between types, so we don't need suffix rules, etc.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I see! So that's what it was for 😅


clean: ## Cleans (deletes) docs and executables
rm -rf docs
rm bin/*
MYST_CLI = src/myst_cli.cr

# https://gist.github.com/prwhite/8168133 "Help target"
help: ## Show this help.
@fgrep -h "##" $(MAKEFILE_LIST) | fgrep -v fgrep | sed -e 's/\\$$//' | sed -e 's/##//'
MYST_INTERPRETER_SPEC = spec/all_spec.cr
MYST_IN_LANG_SPEC = spec/myst/spec.mt
Copy link
Member

Choose a reason for hiding this comment

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

spaces please :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh my, will do


SOURCE_FILES := $(shell find src -type f -name '*.cr')
SPEC_FILES := $(shell find spec -type f -name '*.cr' -o -name '*.mt')
STDLIB_FILES := $(shell find stdlib -type f -name '*.mt')
ALL_FILES := $(SPEC_FILES) $(STDLIB_FILES) $(SOURCE_FILES)

INSTALL_LOCATION ?= /usr/local/bin/myst
Copy link
Member

Choose a reason for hiding this comment

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

Can this default to something like myst-local instead of just myst? I don't want it to conflict with other installations the user might have (e.g., through homebrew).

Copy link
Member Author

Choose a reason for hiding this comment

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

My thoughts was just that we could use this in the actual homebrew installation though (e.g. system "make", "install").

MYSTBUILD ?= myst

O ?= bin
Copy link
Member

@faultyserver faultyserver Mar 13, 2018

Choose a reason for hiding this comment

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

Is O supposed to represent the "object" directory? I'm not a fan of single character names, so I'd like it if this could be expanded to something more descriptive, or at least have a comment about what it represents.

Copy link
Member Author

Choose a reason for hiding this comment

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

O is supposed to represent 'out', i'll just change it to that, this is how crystal's makefile does it, so I just thought they knew best. But, I agree


info ?= true

# If another default is intentionally wished for,
# set it in local/*.mk, like this:
# .DEFAULT_GOAL = goal
#
.DEFAULT_GOAL ?= spec

info_log = $(if $(subst false,,$(info)),$(info $(1)))

# Makefile convention
all: $(O)/myst $(O)/spec

.PHONY: spec
spec: $(O)/myst $(O)/spec ## Runs all specs
$(call info_log,Running interpreter spec)
$(O)/spec
$(call info_log,Running in-language spec)
$(O)/myst $(MYST_IN_LANG_SPEC)

myst-spec: $(O)/myst ## Runs just the in-language specs
$(O)/myst $(MYST_IN_LANG_SPEC)

.PHONY: install
install: $(INSTALL_LOCATION) ## Install myst to INSTALL_LOCATION

.PHONY: build
build: $(O)/myst ## Builds myst into an executable

.PHONY: myst-spec_with_build
myst-spec_with_build: ## Runs the in-language specs with MYSTBUILD
$(MYSTBUILD) $(MYST_IN_LANG_SPEC)

$(O)/myst: $(SOURCE_FILES)
$(call info_log,Building myst...)
mkdir -p $(O)
crystal build -o $(O)/myst $(MYST_CLI)
Copy link
Member

Choose a reason for hiding this comment

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

I think using shards build for this might be more consistent, rather than relying on $(O) to point to the right place. Not a requirement, just a thought :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought about that. OUT (which is what i'll change O to) can be set like this make build OUT=.../somewhere, so im pretty sure OUT will be the right place, and $(OUT)/myst is required to be a file for the $(SOURCE_FILES) dependency to work.

Copy link
Member Author

@Jens0512 Jens0512 Mar 13, 2018

Choose a reason for hiding this comment

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

So if you want shards build then just use shards build, not make build is my point. :)


$(O)/spec: $(ALL_FILES)
$(call info_log,Building specs...)
mkdir -p $(O)
crystal build -o $(O)/spec $(MYST_INTERPRETER_SPEC)

$(INSTALL_LOCATION): $(SOURCE_FILES)
$(call info_log,Installing myst to $(INSTALL_LOCATION) ...)
mkdir -p $(dir $(INSTALL_LOCATION))
sudo crystal build --release -o $(INSTALL_LOCATION) $(MYST_CLI)

.PHONY: clean
clean: ## Cleans (deletes) docs and executables
ifdef O
@[ -f $(O)/myst ] && rm -f $(O)/myst*
@[ -f $(O)/spec ] && rm -f $(O)/spec*
endif
@rm -rf docs
@

# Thanks https://stackoverflow.com/questions/4219255/how-do-you-get-the-list-of-targets-in-a-makefile/26339924#26339924
define TARGET_LIST =
$(shell \
$(MAKE) -pRrq -f $(MAKEFILE_LIST) : 2>/dev/null | awk -v RS= -F: '/^# File/,/^# Finished Make data base/ {if ($$1 !~ "^[#.]") {print $$1}}' |\
sort |\
egrep -v -e '^[^[a-zA-Z0-9]]' -e '^$@$$' |\
xargs )
endef

# Used in help target
list:
@echo $(TARGET_LIST)

HELP_INDENTATION ?= 2
HELP_NAME-DESC_SEP ?= 2

LENGTH_OF_LONGEST_TARGET_NAME = $(shell ruby -e 'puts ARGV.sort_by(&:size)[-1].size' $$(make list))

parse_help = perl -n -e \
'if(/^(\S*)?(?=:).*$(HELP_MARK)\s*(.*)/){ \
print((" " x $(HELP_INDENTATION)) . "$$1" . (" " x ($(LENGTH_OF_LONGEST_TARGET_NAME) + $(HELP_NAME-DESC_SEP) - length($$1))) . "- $$2\n") \
}' $(1) |\
sort

# Help target that reads text after two '#'s in a target definition and prints it after the target name
.PHONY: help
help: ## Show this help
@echo "The main targets are:"
@$(call parse_help, $(firstword $(MAKEFILE_LIST))); echo
# Iterate through the extra optional makefiles
@for makefile in $(wordlist 2, $(words $(MAKEFILE_LIST)), $(MAKEFILE_LIST)); do
echo "Targets defined in $$makefile:"
$(call parse_help, $$makefile)
echo
done
@echo "The default target is '$(.DEFAULT_GOAL)'"

define HELP_MARK
##
endef
1 change: 1 addition & 0 deletions spec/all_spec.cr
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
require "./**"
2 changes: 1 addition & 1 deletion src/myst/cli.cr
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require "option_parser"

module Myst
class Cli
class CLI
def self.run
source = ""
show_ast = false
Expand Down
2 changes: 1 addition & 1 deletion src/myst_cli.cr
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ require "./myst"

include Myst

Cli.run
CLI.run