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

An attempt to optionally use the compile server in erlc. #902

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mbj4668
Copy link
Contributor

@mbj4668 mbj4668 commented Sep 7, 2020

We use the env var ERLC_USE_SERVER to control the behaviour, since
erlc already uses that variable.

Although this seems to work fine for our project, I am not sure that
it works correctly with files generated from .yrl etc.

We use the env var ERLC_USE_SERVER to control the behaviour, since
erlc already uses that variable.

Although this seems to work fine for our project, I am not sure that
it works correctly with files generated from .yrl etc.
@essen
Copy link
Member

essen commented Oct 29, 2020

Yeah it definitely doesn't work for generated files.

@mbj4668
Copy link
Contributor Author

mbj4668 commented Oct 29, 2020

Yes. I did some more fixes so that things like eunit works, but I think that you probably will do a better (correct) implementation anyway, so I think you can ignore this PR. (That said, it works fine for our needs!)

@essen
Copy link
Member

essen commented Oct 29, 2020

It's tough to get right so any help is appreciated.

@mbj4668
Copy link
Contributor Author

mbj4668 commented Oct 29, 2020

Ok, I pushed the other changes we did. In our project we have additional generated erl files (which we append to ERL_FILES), and it seems to work.

@essen
Copy link
Member

essen commented Oct 29, 2020

Could you provide the Makefile related to the generated files? Or at least the part directly related to that + where the include erlang.mk line is located with regard to that.

@mbj4668
Copy link
Contributor Author

mbj4668 commented Oct 30, 2020

Sure, here's the relevant snippet of the Makefile.

ERLC_USE_SERVER = true
export ERLC_USE_SERVER

EUNIT_OPTS = verbose,{print_depth,9999},{report,{eunit_surefire,[{dir,"."}]}}

include $(TOP_DIR)/mk/erlang.mk

ebin/$(PROJECT).app:: $(wildcard $(PROJECT_ENV_FILE))

# there is no variable LOCAL_BUILD_DEPS in erlang.mk, so we fake it here
ALL_DEPS_DIRS += $(addprefix $(LIB_DIR)/, $(LOCAL_BUILD_DEPS))

# ensure we have correct include path
ERLC_OPTS += -I $(LIB_DIR)
TEST_ERLC_OPTS += -I $(LIB_DIR)

# we allow export of variables from if/case
ERLC_OPTS := $(filter-out +warn_export_vars,$(ERLC_OPTS))
TEST_ERLC_OPTS := $(filter-out +warn_export_vars,$(TEST_ERLC_OPTS))

## add targets for yang compilation

YANG_FILES = $(filter %.yang,$(ALL_SRC_FILES))
YANG_ERL_FILES = $(patsubst src/%.yang, src/yy-%.erl, $(YANG_FILES))
ERL_FILES += $(YANG_ERL_FILES)
YANG_BEAM_FILES = $(patsubst src/%.yang, ebin/yy-%.beam, $(YANG_FILES))

app:: $(YANG_BEAM_FILES)

YANGER_OPTS ?=

YANGER_YY_DIR = $(LIB_DIR)/yanger_yy/ebin
YY_SRC = $(LIB_DIR)/yy/src
YY_DIR = $(LIB_DIR)/yy/ebin
SYST_SRC = $(LIB_DIR)/syst/src

yang_verbose_0 = @echo " YANGER " $(filter %.yang,$<);
yang_verbose_2 = set -x;
yang_verbose = $(yang_verbose_$(V))

src/yy-%.erl: src/%.yang ../yy/include/yy.hrl $(YANGER_YY_DIR)/yanger_yy.beam
	$(yang_verbose) $(YANGER) -Werror -P $(YANGER_YY_DIR) --pz $(YY_DIR) \
	-p $(SYST_SRC) -p $(YY_SRC) -f yy $(YANGER_OPTS) -o $@ $<

## if we have yang files we need yanger
ifneq ($(strip $(YANG_FILES)),)
LOCAL_BUILD_DEPS += yanger_yy
endif

@essen
Copy link
Member

essen commented Oct 30, 2020

Thanks! I'll take a look when time allows. Plan is to do this and package changes together as they're both breaking or potentially breaking changes.

@mbj4668
Copy link
Contributor Author

mbj4668 commented Oct 30, 2020

Ok. +1 to the package changes as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants