search

craftr-build / craftr-build-4.x

Frontend for the Craftr build framework.
https://craftr-build.github.io/craftr/
Other
60 stars 14 forks source link

Support CFLAGS, CPPFLAGS and LDFLAGS #111

Closed NiklasRosenstein closed 7 years ago

NiklasRosenstein commented 8 years ago

The C/C++ compiler interfaces should support CFLAGS, CPPFLAGS and LDFLAGS environment variables and automatically add them as additional compilation flags. However, there should also be an option to ignore these environment variables, eg:

lib = ld.link(
  output = 'main',
  output_type = 'dll',
  include_env_flags = False,  # True by default
)
winksaville commented 8 years ago

Bravo, especially the ignore flag. What is the default? Is there a way to change the default locally & globally (options maybe)?

On Mon, May 23, 2016, 9:02 AM Niklas Rosenstein notifications@github.com wrote:

The C/C++ compiler interfaces should support CFLAGS, CPPFLAGS and LDFLAGS environment variables and automatically add them as additional compilation flags. However, there should also be an option to ignore these environment variables, eg:

lib = ld.link( output = 'main', output_type = 'dll', include_env_flags = False, # True by default )

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/111

NiklasRosenstein commented 8 years ago

These flags would be read the same way that options are read with craftr.options.get(): Eg. for CFLAGS, it will first check if there is an environment variable module_name.CFLAGS and if there is not, it will look out for the global CFLAGS. Thus you can unset the CFLAGS on a per-module basis by doing

craftr -e -D module_name.CFLAGS=

or using a relative identifier from the main module

craftr -e -D .CFLAGS=
NiklasRosenstein commented 8 years ago

The default would be to read the CFLAGS. If no CFLAGS are set or the variable is empty, no additional arguments are added.

winksaville commented 8 years ago

I worry about magic like that, I like to be in control. Is there a command line flag to display these magical options?

On Mon, May 23, 2016, 9:54 AM Niklas Rosenstein notifications@github.com wrote:

The default would be to read the CFLAGS. If no CFLAGS are set or the variable is empty, no additional arguments are added.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/111#issuecomment-221030055

NiklasRosenstein commented 8 years ago

Could you explain on what part of this appears magical to you? Just to avoid misunderstandings. Thanks

winksaville commented 8 years ago

Acquiring information outside side of the craftr files is magical to me. For instance I doing a demo on someone else's computer and CFLAGS is set and I don't realize it is likely the behavior will not be the expected one. That's the type of thing I'm referring to.

On Mon, May 23, 2016, 10:05 AM Niklas Rosenstein notifications@github.com wrote:

Could you explain on what part of this appears magical to you? Just to avoid misunderstandings. Thanks

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/111#issuecomment-221032890

NiklasRosenstein commented 8 years ago

I see where you're coming from, and I also have the concerns that some arbitrary CFLAGS environment variable might be floating around somewhere that gets taken into consideration although that is unwanted/unexpected by the developer.

I wonder though, isn't that the way that all other build tools behave like? @dfroger also mentioned possible issues with package managers that expect CFLAGS to be taken into account.

Of course, it would be possible to take a middle ground: There could be another environment variable that is considered globally, something that turns on consideration of CFLAGS etc. like

craftr -e -D CRAFTR.EXFLAGS

Now if CRAFTR.EXFLAGS is set, CFLAGS etc. would be taken into account by default, otherwise they would be ignored by default. It could also be made into a special command-line option like craftr --exflags or the inverse craftr --no-exflags

What do you think?

winksaville commented 8 years ago

I like your proposals.

And yes, IIRC, the default for make is to use those environment variables and a bunch of default rules, but it is easy to disable via 'SUFFIXES:'. Personally I tend to use SUFFIXES, and I think its better to enable "magic" then have to turn it off. Of course YMMV.

On Mon, May 23, 2016, 10:38 AM Niklas Rosenstein notifications@github.com wrote:

I see where you're coming from, and I also have the concerns that some arbitrary CFLAGS environment variable might be floating around somewhere that gets taken into consideration although that is unwanted/unexpected by the developer.

I wonder though, isn't that the way that all other build tools behave like? @dfroger https://github.com/dfroger also mentioned possible issues with package managers that expect CFLAGS to be taken into account.

Of course, it would be possible to take a middle ground: There could be another environment variable that is considered globally, something that turns on consideration of CFLAGS etc. like

craftr -e -D CRAFTR.EXFLAGS

Now if CRAFTR.EXFLAGS is set, CFLAGS etc. would be taken into account by default, otherwise they would be ignored by default. It could also be made into a special command-line option like craftr --exflags or the inverse craftr --no-exflags

What do you think?

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/111#issuecomment-221040441

dfroger commented 8 years ago

I agree that environment variable can be dangerous. It is frequently the case that someone defines environment variable for fixing a problem, and then forgot them, which could then break a build if craftr "magically" take them into account.

In the video about cross compiling, a BSD package manager give its point of view on build system.

Here is how meson manage environment variables. Meson take them into account only if the flag --buildtype=plain is passed. I like this feature.

I'll try to understand how CMake manage this.

Another thing I'm wondering is if craftr should give the possibiliy to completely replace C flags set by the craftfile, or just prepend values to them, or both possibility?

dfroger commented 8 years ago

CMake seems to take into account CFLAGS environment variable:

dfroger commented 8 years ago

This is how it works in automake:

NiklasRosenstein commented 8 years ago

@winksaville but it is easy to disable via 'SUFFIXES:'.

Could you elaborate on that? I did use Make for some time, but the only thing SUFFIXES tells me is the $(addsuffix)function. :)

@dfroger Thanks for the links!

I do like the way Meson does it with --buildtype=plain. I wonder though if using this plain mode will use only the environment variable flags or whether they are only added to the flags that Meson would use.

Because when I opened this issue, I did not think about CFLAGS actually replacing the flags that Craftr would add by default, but only adding them (as if you passed them to the additional_flags argument). I think you where asking kind of the same question?

Another thing I'm wondering is if craftr should give the possibiliy to completely replace C flags set by the craftfile, or just prepend values to them, or both possibility?

So both CMake and automake don't seem to have a way to disable using CFLAGS.

NiklasRosenstein commented 8 years ago

Do you know how other build tools handle the linking step? Because there's LD and LDFLAGS, but currently Craftr just invokes the compiler as a linker as it usually adds a lot of necessary flags to ld that are hard to figure out!

NiklasRosenstein commented 8 years ago

So basically what I wonder is this: If LDFLAGS is used, is it intended to be passed to the linker or to the compiler that wraps the linker invokation? Ie. can I expect the LDFLAGS to be in the correct format (ie. prefixed with -Wl,) or do I manually have to convert it to prefix -Wl, ? This would mainly be an issue with package managers since a human could just choose the right format of the flags.

Just in case my question isn't clear enough, since I'm having a hard time phrasing it, here's an example:

Would you use/do package managers use

NiklasRosenstein commented 8 years ago

For now I'll just assume LDFLAGS are arguments that would be passed directly to ld and convert them to -Wl, format on the fly. The changes are in the buildtype-support branch for now.

A --buildtype argument was added which accepts the values standard and external. External build mode adds the flags in CFLAGS, CPPFLAGS and LDFLAGS to the existing ones.

winksaville commented 8 years ago

My opinion is meason's build type=plain is to obtuse, I suggest an explicit USE_ENVs. For empty SUFFIXES see last few paragraphs here: http://www.gnu.org/software/make/manual/html_node/Suffix-Rules.html

On Wed, May 25, 2016, 3:29 PM Niklas Rosenstein notifications@github.com wrote:

For now I'll just assume LDFLAGS are arguments that would be passed directly to ld and convert them to -Wl, format on the fly. The changes are in the buildtype-support https://github.com/craftr-build/craftr/tree/buildtype-support branch for now.

A --buildtype argument was added which accepts the values standard and external. External build mode adds the flags in CFLAGS, CPPFLAGS and LDFLAGS to the existing ones.

— You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub https://github.com/craftr-build/craftr/issues/111#issuecomment-221727204

NiklasRosenstein commented 8 years ago

To be honest I quite like it, although I changed the argument name from plain to external which makes more sense IMHO, as the build is (partially) controlled externally via environment variables.

It would look like craftr --buildtype=external (or use the shorthand -t external).

I think the SUFFIXES thing does not translate to Craftr as there are no built-in rules (in fact there are no rules at all, just "generator functions" that generate targets with high-level interface, updated that in the documentation as well).

Do you think its correct to just add the CFLAGS to the command-line or should they completely (or partially??) replace the arguments Craftr would be using? (By default, Craftr only uses a few arguments anyway such as enabling warnings and enabling exceptions by default on Windows)

dfroger commented 8 years ago

Sorry for the late reply...

Because when I opened this issue, I did not think about CFLAGS actually replacing the flags that Craftr would add by default, but only adding them (as if you passed the m to the additional_flags argument). I think you where asking kind of the same question?

Yes, this is what I where asking.

The automake doc say in Flag variable ordering:

The reason ‘$(CPPFLAGS)’ appears after ‘$(AM_CPPFLAGS)’ or
‘$(mumble_CPPFLAGS)’ in the compile command is that users should always have
the last say. It probably makes more sense if you think about it while looking
at the ‘CXXFLAGS=-O0’ above, which should supersede any other switch from
AM_CXXFLAGS or mumble_CXXFLAGS (and this of course replaces the previous value
of CXXFLAGS).

So the best behaviour may be to have CFLAGS (or other variable) appended to the variables Craftr set.

Do you know how other build tools handle the linking step? Because there's LD and LDFLAGS, but currently Craftr just invokes the compiler as a linker as it usually adds a lot of necessary flags to ld that are hard to figure out!

I would that that gcc is always invoked as a linker for linking executable and dynamic library, and ar is invoked for static libraries, so LDFLAGS is passed to gcc .

-lm and -L/path/to/lib can be passed directly to gcc. Some other flags required -Wl, for example -rpath.

A --buildtype argument was added which accepts the values standard and external. External build mode adds the flags in CFLAGS, CPPFLAGS and LDFLAGS to the existing ones

Great!

dfroger commented 8 years ago

The make documentation on implicit variable says:

LDFLAGS    Extra flags to give to compilers when they are supposed 
to invoke the linker, ‘ld’, such as -L. Libraries (-lfoo) should be added 
to the LDLIBS variable instead.

So LDFLAGS is passed to the compiler, which invokes the linker.

And there are two environemnts variables taken into account : LDFLAGS and LDLIBS. I don't know how much it is important to distinguish them...

NiklasRosenstein commented 8 years ago

So the best behaviour may be to have CFLAGS (or other variable) appended to the variables Craftr set.

Thanks for finding that piece of information, David.

-lm and -L/path/to/lib can be passed directly to gcc. Some other flags required -Wl, for example -rpath.

I think checking if LDFLAGS starts with -Wl, and otherwise convert them to -Wl, format would be the best to support both cases. And I'll let Craftr add LDLIBS to the compiler command directly.

The arguments from the environment variables will always be appended to the end of the argument list, even after the additional_flags option.

Regarding the --buildtype, please take a look at the documentation: http://craftr.readthedocs.io/en/latest/cmd.html#t-buildtype-standard-external @winksaville @dfroger

edit: fixed link target

NiklasRosenstein commented 7 years ago

Closing in favour of #152