MakeDeps: Provide "require" information, and more styling tweaks #14605
Conversation
conan/tools/gnu/makedeps.py
Outdated
| @@ -507,8 +512,9 @@ class DepGenerator: | |||
| Process a dependency cpp_info variables and generate its Makefile content | |||
| """ | |||
|
|
|||
| def __init__(self, dependency): | |||
| def __init__(self, dependency, require=None): | |||
There was a problem hiding this comment.
If the require is always passed, better remove the None default
There was a problem hiding this comment.
Whoops, minor oversight there. Was thinking of making it optional at first.
conan/tools/gnu/makedeps.py
Outdated
| @@ -597,6 +603,7 @@ class MakeDeps: | |||
| """ | |||
|
|
|||
| _title = "# This Makefile has been generated by Conan. DO NOT EDIT!\n" | |||
| _footer = f"# end {CONAN_MAKEFILE_FILENAME}\n" | |||
There was a problem hiding this comment.
Not sure about the value of this footer. No other Conan generated file adds this end-of-file footer, so better remove it to align.
There was a problem hiding this comment.
I'll remove it, but I would advise adopting this as a general convention. A clear footer indicates that you have the entire file, that it was not truncated for one reason or another.
There was a problem hiding this comment.
We haven't been reported a truncated file ever in many years. Creation of generated files happen in memory, and written all at once in single operation, extremely unlikely to happen.
conan/tools/gnu/makedeps.py
Outdated
| CONAN_NAME_{{ name }} = {{ dep.ref.name }} | ||
|
|
There was a problem hiding this comment.
I removed these lines, because I like visually more compact files, and group all similar things in blocks. For me, all the name, version, ref, are conceptually part of the same "block" of information, and I like to have some visual hints of these information groups.
There was a problem hiding this comment.
Okay, I'm glad to see there is thought behind this. I'll leave the blank right before _ROOT, as that relates more to the other variables.
iskunk
force-pushed
the
feature/makedeps-tweak
branch
from
875c2b6 to
21fc098
Compare
|
Let me know if you have any further concerns with this (updated) patch. |
|
Thanks @memsharded and @uilianries. I'm happy to see this generator finished, merged, and polished off. Looking forward to seeing what you have in mind for MakeToolchain. |
|
Thank you for your words @iskunk !! About the MakeToolchain, your feedback will be very important too! Regards! |
Changelog: Feature: MakeDeps: Provide "require" information, and more styling tweaks.
Docs: Omit
Minor follow-on to #14133
This includes the following:
(indirect dependency)comment on applicable components (just an example of what can be done with therequireinfo);requireinfo toDepContentGeneratorandDepGenerator, as it provides useful context for a dependency;