Fix issue 14188 - implement makefile compatible dependency file#12011
Fix issue 14188 - implement makefile compatible dependency file#12011dlang-bot merged 1 commit intodlang:masterfrom
Conversation
rtbo
requested review from
Geod24,
MartinNowak,
UplinkCoder,
WalterBright and
rainers
as code owners
|
Thanks for your pull request and interest in making D better, @rtbo! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#12011" |
|
changed switch to |
One switch is introduced: -makedeps, which can take an optional filename argument. If filename is omitted, dependencies are printed to stdout. Module dependencies are collected in dmodule.Module.read. Files imported with the -J switch are collected in the expression semantics visitor.
| } | ||
| } | ||
|
|
||
| if (params.makeDeps && !params.oneobj) |
There was a problem hiding this comment.
I think this is unnecessarily restrictive: anything that builds to a single output file should be fine, e.g. a library.
BTW: just checking params.makeDeps later is enough and it would make it easier to extend to other cases.
There was a problem hiding this comment.
anything that builds to a single output file should be fine
This is why I disabled makedeps if oneobj is false, but yes it would be better to test for multiobj instead.
I saw in the logic in the same function that oneobj is set as soon as there is a link operation.
Only for static lib I believe it can be unset.
There was a problem hiding this comment.
These oneobj and multiobj flags sometimes only describe intermediate steps (e.g. when building a library, multiobj is used, but the object files are not actually written to disk). So a slightly more elaborate interpretation of the params might be necessary to support other cases.
The generated object file might also be an intermediate file, e.g. when automatically linking to an executable. I'd expect the executable to be the target of the make dependency. Actually, all generated files could be listed there. For example, if the build system also deals with generated documentation. That could also cover building multiple object files wih -c.
No immediate action required though, this PR was already a good step forward.
| OutBuffer* ob = global.params.makeDeps; | ||
| ob.writestringln(" \\"); | ||
| ob.writestring(" "); | ||
| escapePath(ob, toPosixPath(name)); |
There was a problem hiding this comment.
Whether \ must be converted to / might depend on what the version of make or what your actual makefile uses, but should be ok for most. escapePath escapes ( and ), but that should be unnecessary for make, Instead, $ has to be dealt with.
There was a problem hiding this comment.
OK. I'm definitely not a make guru. I've seen cases where there is a mix of \ and / in the same file path.
I assumed normalizing everything to / would be the safest.
I thought escapePath also escape the spaces. These should probably be escaped otherwise we could have broken dependency files.
|
Thx Remi! - Out of curiosity, are you working on a CMake integration, reggae or the like? |
Yes, I've started to work on Meson support. See https://github.com/rtbo/meson/tree/dlang_depfile |
5 participants
This PR fixes issue 14188 to allow emitting makefile compatible dependency file and enable proper D builds with ninja.
I am aware #6961 and #11357.
IMO #6961 is too complex, introducing many switches and stales for nearly two years.
This one is closer in spirit to #11137, but with simpler and more efficient implementation.
In particular, it follows advice given by @rainers to not use the same semantics than the current
-depsswitch, which do much more than collecting depending modules (it triggers additional semantic pass).Only one switch is introduced:
-makefiledeps, which can take an optional filename argument. if filename is omitted, dependencies are printed to stdout.I collect module dependencies in
dmodule.Module.read.I collect in addition imported files (with the
-Jswitch) in the expression semantics visitor.Nothing else is collected or analysed, compilation time should not be affected more than strictly necessary.
I've tested this to work with Ninja.