-
Notifications
You must be signed in to change notification settings - Fork 364
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add docu for deployers and a simple example #2946
Add docu for deployers and a simple example #2946
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good job
|
||
|
||
Inspecting the command output we can see that it copied the sources of all our dependencies (direct and transitive) | ||
to a ``dependency_sources`` folder. After this, extra preprocessing could be accomplished to more specific needs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dependencies_sources
might be better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where is this dependency_sources
folder located?
deploy() | ||
++++++++ | ||
|
||
The ``deploy()`` method is called by Conan, and gets both a dependency graph (``conans.client.graph.graph.DepsGraph``) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better not document a private object yet (nothing in the conans
namespace should be in the docs at all), because users might thing they can instantiate or use their methods. Until we document the API of the graph, better treat it as opaque as possible, just saying that it has a root.conanfile
would be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if consumer receives some sort of opaque object, they need to know that are the valid methods/attributes to be safely called.
if object itself cannot be documented, then it should define some sort of an interface.
e.g. how do I know that graph
has graph.root
, and graph.root
has graph.root.conanfile
, and graph.root.conanfile
has graph.root.conanfile.dependencies
that is some sort of dict.
how do I know that are these object and are they stable, if they are never explained?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment, it is enough to say that graph.root.conanfile
is available, and that has the documented dependencies
attribute, which is good for most cases. Let's just document that, but not the whole object private interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and documentation on the dependencies interface says explicitly: This information does not exist in some recipe methods, only in those methods that evaluate after the full dependency graph has been computed.
, At the moment, this information should only be used in generate() and validate() methods. Any other use, please submit a Github issue.
. it doesn't seem to say it's available in deploy()
method and allowed to be used there. that's a bit contradictory from the documentation to say in one place it's not allowed, and in another it's allowed 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are referencing Conan 1.X docs. The docs for 2.0 will be (and already are) very different to those of 1.X. Deployers is a 2.0 feature, and this PR is for 2.0-beta
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says the same in 2.0 docs: https://docs.conan.io/en/2.0/reference/conanfile/other/dependencies.html?highlight=dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says the same in 2.0 docs: https://docs.conan.io/en/2.0/reference/conanfile/other/dependencies.html?highlight=dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, yes, that would be a bug in the docs, seems it was a copy&paste from 1.X
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
.. code-block:: python | ||
|
||
from conan.tools.files import copy | ||
import os |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think, as it's for conan 2 and python 3+, it may already use Pathlib
|
||
Deployers run before generators, and they can change the target folders. | ||
For example, if the ``--deploy=full_deploy`` deployer runs before ``CMakeDeps``, | ||
the files generated by ``CMakeDeps`` will point to the local copy in the user folder done by the ``full_deploy`` deployer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do they need to affect on generators behavior? that's super confusing part.
as a consumer, I'd expect deployers just to deploy (copy) things, and otherwise, leave other parts untouched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is one of most requested behaviors for deployers and generators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's kinda surprising behavior that deployers may modify files generated by generators. can it be made more explicit? e.g. --deploy=full_deploy --deploy-modify-generator=CMakeDeps
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some things that are not covered:
- can I specify multiple deployments at once (
--deploy=a --deploy=b
), or should I use separatedconan install
commands for each of the invocations? - what happens if
deploy()
raises in the middle of deployment? will it leave some intermediate state or roll back? will it continue to execute other deployers, if any?
.. code-block:: bash | ||
|
||
$ git clone https://github.com/conan-io/examples2.git | ||
$ cd examples2/examples/extensions/deployers/sources |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get:
cd: no such file or directory: examples2/examples/extensions/deployers/sources
is it expected? or typo in path?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is expected as the PR in examples2 has not yet been merged
|
||
Such that every dependency will end up in a folder such as: | ||
|
||
``[OUTPUT_FOLDER]/host/dep/0.1/Release/x86_64`` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as it doesn't include all the settings/options, is there a risk that several deployments will overwrite each other? maybe it's better just to use package id here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is, but if the user is worried about such problems, they are encouraged to write their own deployer, but let's tag @memsharded in case he thinks this could be nice to have
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
|
||
Deployers run before generators, and they can change the target folders. | ||
For example, if the ``--deploy=full_deploy`` deployer runs before ``CMakeDeps``, | ||
the files generated by ``CMakeDeps`` will point to the local copy in the user folder done by the ``full_deploy`` deployer, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is kinda unspecified, what happens in case of multiple deployers, if all of them want to change target directories, what happens?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, the last one would win
Co-authored-by: SSE4 <[email protected]>
Co-authored-by: SSE4 <[email protected]>
Co-authored-by: SSE4 <[email protected]>
Co-authored-by: SSE4 <[email protected]>
Co-authored-by: SSE4 <[email protected]>
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
examples/extensions/deployers/sources/custom_deployer_sources.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Co-authored-by: Francisco Ramírez <[email protected]>
Closes #2686
Requires conan-io/examples2#61