Use {CPP,C,CXX}FLAGS from environment to compile foreign files#8464
Use {CPP,C,CXX}FLAGS from environment to compile foreign files#8464glondu wants to merge 1 commit intoocaml:mainfrom
Conversation
src/dune_rules/foreign_rules.ml
Outdated
| | Cxx -> Fdo.cxx_flags ctx | ||
| | Cxx -> | ||
| List.concat | ||
| [ get_env_flags "CXXFLAGS" |
There was a problem hiding this comment.
You can use Env.get instead here. The Env.t value should come from ctx.env.
We should probably also guard this behaviour behind dune lang. cc @rgrinberg wdyt?
There was a problem hiding this comment.
You can use
Env.getinstead here. TheEnv.tvalue should come from ctx.env.
Done.
We should probably also guard this behaviour behind dune lang.
...or behind a command-line flag, maybe? My goal is to make all packages take CFLAGS into account without changing each package... In the current state of matters (in Debian), it is easy to add a command-line flag to all calls to dune build. But I am not aware of a way to update all packages to a new dune lang at once (and with my current understanding, it cannot be done blindly).
cc @rgrinberg wdyt?
What does "wdyt" mean?
There was a problem hiding this comment.
Guarding it behind a command line flag is fine. Do you use a custom workspace file when building dune packages in debian? Putting it there is fine too.
There was a problem hiding this comment.
Do you use a custom workspace file when building dune packages in debian?
I was not familiar with this notion, so no. The way most dune packages are built in debian is defined here... Sorry, it's Perl, but should be understandable: the interesting parts are the last four functions (build, test, install and clean).
There was a problem hiding this comment.
Guarding it behind a command line flag is fine.
I've implemented that in a separate commit. I'm wondering if this is the right move, though. The feature could be useful for installing opam packages, too, and there is no way to change the command used there...?
There was a problem hiding this comment.
What does "wdyt" mean?
"What do you think"
7d4c65e to
20cc2fd
Compare
|
We could also check the |
|
@MisterDA Should we add I discussed this with the team and it seems like this feature is the right way to go. To proceed with the PR:
|
|
@MisterDA since you're interested, do you mind reviewing this one? |
Yes! Looking for inspiration, Arch Linux' makepkg (the program used to build custom packages) allows user to set
It looks good, but some tests showcasing this would be nice, indeed. As to whether use the env var by default or not (use the flag), I've looked at Meson Compiler and linker flag environment variables: it supports them by default in the name of backwards compatibility, but recommends against their use and suggest explicit configuration of the project. I don't have a strong opinion, but backwards compatibility is a compelling argument. |
20cc2fd to
20b180f
Compare
|
I added support for |
|
Thanks @glondu. What do you think about supporting LDFLAGS as well? |
See ocaml#8291 Signed-off-by: Stephane Glondu <steph@glondu.net>
20b180f to
82bfee4
Compare
I'm not sure about this one. According to my interpretation of make's built-in rules, they should be used in linking rules (and I would say all of them, even pure OCaml ones), which I haven't located yet (I'm new to dune's codebase). FYI, in Debian package building environment, LDFLAGS is set to |
|
Please re-open if you intend to finish this PR |
See #8291