conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.29k stars 982 forks source link

[bug] Conan center hook overwrites update_conandata() #17264

Closed MartyMcFlyInTheSky closed 2 weeks ago

MartyMcFlyInTheSky commented 3 weeks ago

Describe the bug

Conan 1.61: I'm trying to retrieve the commit hash in the exported package, therefore doing update_conandata() to later (in the generate function) retrieve the hash. Here's my export function:

    def export(self):
        git = Git(self, self.recipe_folder)
        scm_url = git.get_remote_url()
        scm_commit = git.get_commit()
        # scm_url, scm_commit = git.get_url_and_commit()
        # we store the current url and commit in conandata.yml
        print("update conandata")
        update_conandata(self, {"scm": {"commit": scm_commit, "url": scm_url}})

Now the problem is that when I do this and do a conan create . user/testing I will see that the conandata.yaml file in my export folder is empty. This is apparently because the conan-center hook will just empty the file:

[HOOK - conan-center.py] post_export(): [CONANDATA.YML REDUCE (KB-H031)] Saving conandata.yml: {}
[HOOK - conan-center.py] post_export(): [CONANDATA.YML REDUCE (KB-H031)] New conandata.yml contents: {}

Upon removal of the hook (conan config rm hooks.conan-center) the conandata is valid.

Not sure if this behaviour was intended, but it makes the off-the-shelf setup quite awful to work with since now it seems I have to enforce the removal of this hook for our whole devteam.

Also I'm not sure if I'm missing something, but it seems this is the only way I can forward the commit hash of the original repo to cmake and then add it as a C++ definition, right? It has to work with both, conan create . and `conan build .

How to reproduce it

No response

memsharded commented 2 weeks ago

Hi @MartyMcFlyInTheSky

Thanks for your report.

I think the issue is that the ConanCenter hooks are intended exclusively to work with ConanCenter packages. Having them for your own packages is not intended or recommended, they can have indeed some unintended effects like this one.

Are you working both with a conan-center-index fork and your own repos and packages with the same configuration? The vast majority of users that work with a conan-center-index fork have its own build for it, and they will only install the ConanCenter hooks in that build, but not for development of other packages.

MartyMcFlyInTheSky commented 2 weeks ago

Thanks for your answer @memsharded. We do not use a fork of conan-center-index actually, so you're right it doesn't make sense to have the hooks activated. But I think I didn't change the default afaik. Are they on by default then?

In any cased I'll advise my team to remove the hooks.

memsharded commented 2 weeks ago

But I think I didn't change the default afaik. Are they on by default then?

No, hooks aren't even installed with Conan, they need to be installed with conan config install, so they are not there by default.

It is true that there are some hooks that might be useful as quality checks, but as a first step I think you are good removing them, your usual testing of packages and products should certainly cover things more than enough.

I was curious if the modern hook for Conan 2 would have the same effect, but probably doesn't. Because it is based on the https://docs.conan.io/2/reference/tools/files/basic.html#conan.tools.files.conandata.trim_conandata function, which will respect the scm field.

So maybe this ticket can be closed?

MartyMcFlyInTheSky commented 2 weeks ago

Yes thank you very much for the annswer!