Makefile: Allow to add EXTRA_FLAGS to FLAGS

[jirutka]

No description provided.

[RX14]

Why not rename it CRYSTAL_FLAGS and then make it append: CRYSTAL_FLAGS := $(CRYSTAL_FLAGS)...?

By default FLAGS is empty so "set FLAGS and don't set anything else" is an override but you can optionally set stuff like release and have both.

[Jens0512]

Would it not to be better to just do something like

override FLAGS += $(if $(release),--release )$(if $(stats),--stats )$(if $(progress),--progress )$(if $(threads),--threads $(threads) )$(if $(debug),-d )$(if $(static),--static )$(if $(LDFLAGS),--link-flags="$(LDFLAGS)" )

That way specifying make all FLAGS=--totally-existant-flag would just prepend --totally-existant-flag to the old FLAGS. And you wouldn't have to type the extra EXTRA_

[RX14]

Yeah thats what I meant, I should learn more makefile...

[jirutka]

That way specifying make all FLAGS=--totally-existant-flag would just prepend --totally-existant-flag to the old FLAGS. And you wouldn't have to type the extra EXTRA_

That indeed an option, but it would not be backward compatible – now you can completely override FLAGS. If Crystal maintainers are okay with this change of behaviour, I’ll change it to the suggested variant.

[jhass]

I had the same concern at first but then there are actually no default values in FLAGS, all depend on another variable being set. So previously specifying FLAGS and for example release didn't work, you already had to do FLAGS=--release --whatever, which will continue to work just the same. Only if you would have specified release=1 FLAGS=--release there would be a slight change, but that was kinda invalid previously already.

[Jens0512]

With my suggested change, make all release=1 FLAGS=--no-debug would build with the flags --no-debug --release

[jirutka]

@Jens0512 Is override really necessary? I think that FLAGS += … should work too; what difference does it make?

[Jens0512]

@jirutka Override is absolutely necessary

[Jens0512]

@jirutka See RX14s, and my comment for why.

[RX14]

If a variable has been set with a command argument (see Overriding Variables), then ordinary assignments in the makefile are ignored.

From the make docs.

This change currently actually does nothing, as += doesn't make sense without override in this case.

[Jens0512]

@jirutka Without override, the += does nothing when FLAGS=... is passed to make, as Chris said. So make all release=1 FLAGS=--time would run with just the flag --time. In other words, += works just the same as := in this case, and nothing will really be changed from how it initially was.

[Jens0512]

As to the suggestion to rename FLAGS. If FLAGS should be renamed, following Make convention; it should be renamed to CRFLAGS, not CRYSTAL_FLAGS. Though that’s just the Make convention.

[Jens0512]

This does absolutely nothing. An override is needed.

[RX14]

@Jens0512 yes - we realise that. It doesn't mean @jirutka has had time to update the PR. Please be patient.

[RX14]

@jirutka ping? If you don't have time for this I can do it myself.

[jirutka]

Sorry for delay, I’ve updated it now.