Updated reuse_cmake_install with an example of package()

[mristin]

This update is related to the issue #716 .

[CLAassistant]

All committers have signed the CLA.

[danimtb]

Hi @mristin, thanks a lot for this PR! Could you please check the CLA sign? We need it to get this PR accepted.

Regarding the changes in this PR, one of the things that made us not kind of recommend this usage is because package() method would be called twice in case you define no_copy_source = True.

Also, as conanfile methods are executed independently, you have to create a new CMake object in package() and configure same definitions, for example: https://github.com/bincrafters/conan-mbedtls/blob/414f0a54ba604f5fcd56b85a03874b717ae5036b/conanfile.py#L54

So finally I would say it is a trade-off. And I'd say that if we are going to make this change, it will be better to include the whole solution as in the example linked above.

[mristin]

@danimtb thank you for the review!

Please apologize for somewhat amateurish pull requests -- I'm new to conan and we have just started to use it in the company.

Regarding the changes in this PR, one of the things that made us not kind of recommend this usage is because package() method would be called twice in case you define no_copy_source = True.

After reading the docs you pointed me to I'm a bit puzzled why the package() needs to be called twice. Wouldn't CMake (i.e. cmake.install()) be responsible for all the copying including the headers or any other source files? Could you please give me a bit more details so that I can also update the docs in this pull request? I suppose this merits at least a short paragraph.

Also, as conanfile methods are executed independently, you have to create a new CMake object in package() and configure same definitions, [...]

Could you please have a look at the commit 0675a46 and let me know if that is what you had in mind or something more complex?

[danimtb]

Hi @mristin,

Don't worry, every PR (especially to the docs) is always welcomed and we really appreciate the feedback from users.

Regarding to the commit you pointed: yes, that is exactly what I was trying to highlight.

For the case of package() method being called twice, this happens when you have no_copy_source = True. First, let me explain how Conan behaves:

• Conan retrieves the sources from exports, exports_sources & source() and puts them in a source folder.
• Before compiling, Conan creates a copy of the source folder into the build folder
• Then the build starts.
• After the build Conan creates the package collecting the files and artifacts from the build folder (using package() method of the recipe).

Now, if you set no_copy_source = True Conan behaves this way:

• Retrieve sources (same as above).
• Build starts in the build folder (sources are in source folder -NOT copied to build folder-
• After the build, Conan creates the package copying files from the source folder (this calls package()) and copying the artifacts from the build folder (this calls package() again).

So in the case you were using cmake.install() in the package method and no_copy_source, the install step will be done twice. Files will be overwritten with issues but install steps are sometimes time expensive and this will double the "packaging" time.

[mristin]

@danimtb thanks for your explanation!

I'm still a bit puzzled about this step:

After the build, Conan creates the package copying files from the source folder (this calls package()) and copying the artifacts from the build folder (this calls package() again).

Why would you call the same function twice that doesn't include any parameter to change its behavior? I looked into:
https://github.com/conan-io/conan/blob/c9df68faf57003dbe480f5dbbbe887ec8aeed4a9/conans/model/conan_file.py#L247

    def package(self):
        """ package the needed files from source and build folders.
        E.g. self.copy("*.h", src="src/includes", dst="includes")
        """
self.output.warn("This conanfile has no package step")

so there seems to be no side effects. Are there parameters I'm not aware of (like package_folder dynamically changing between the two steps in the concrete instance of ConanFile)?

(After some discussion in the office) The way I understand CMake is that make install also copies the headers. For example, see https://stackoverflow.com/a/40285224. So if CMake is set up correctly, package() with cmake.install() would be the expected behavior. You don't want to copy the implementation files (i.e. *.cpp), only the headers (i.e. *.h) -- what am I still missing? :)

[mristin]

Hi @danimtb,
Just a kind reminder in case you missed the last message.

[danimtb]

Hi @mristin,

As I told you before (sorry about the mistake):

So in the case you were using cmake.install() in the package method and no_copy_source, the install step will be done twice. Files will be overwritten withoOUT issues but install steps are sometimes time expensive and this will double the "packaging" time.

Currently I thinks the best would be to make the changes you are suggesting in this PR (cmake.install() in package()) and include a warning message regarding no_copy_source. For the future I think we should improve this in the conan client regarding other build helpers too.

[mristin]

Hi @danimtb ,
I tried to write down what we discussed here regarding the different work flows when no_copy_source is set.

I also added a more verbose paragraph on package_info(). That part was also not clear to me, and having the redundant explanation why it is needed (aside the main package_info() page) would help understanding, IMO.

I removed the example that leverages internal conan API.

[danimtb]

Thanks for the re-work @mristin!

[memsharded]

There are a couple of minor issues that could be improved, @danimtb could you please have a look?

[memsharded]

I think this cmake.definitions should be before cmake.configure(), otherwise it is ignored.

[memsharded]

These are the defaults, it is not necessary to define them, and it might be confusing to do so, without a clear warning or something.

[danimtb]

@memsharded done! https://docs.conan.io/en/latest/howtos/cmake_install.html

• Fixed definitions.
• Changed package_id()
• Removed redundant information.
• Linked to no_copy_source reference