Skip to content
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

[question] Generate() vs. Imports() behavior #17700

Closed
1 task done
valenotary opened this issue Feb 4, 2025 · 14 comments
Closed
1 task done

[question] Generate() vs. Imports() behavior #17700

valenotary opened this issue Feb 4, 2025 · 14 comments
Assignees
Labels
responded Responded by Conan team type: question

Comments

@valenotary
Copy link

What is your question?

We are in the middle of migrating over to Conan2 -- we originally had some method override for the imports() method that we naively just moved to generate(). However, we're noticing now that the generate() method is not being executed (i.e. just for debugging, we tried putting some output logs in the generate method and just saw they werent being printed)... Are there any glaring pieces I might be missing in my understanding of how to migrate this/use generate() properly? Maybe my usecase might need some additional steps beyond changing just the method name?

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded memsharded self-assigned this Feb 4, 2025
@memsharded
Copy link
Member

Hi @valenotary

Thanks for your question

The generate() method executes for every conan install command in the "consumer" side, but also before every build() in the Conan cache.

Could you please provide something that we can reproduce on our side? Like some minimal conanfile.py and the commands to reproduce? Thanks for the feedback!

@valenotary
Copy link
Author

valenotary commented Feb 4, 2025

@memsharded thanks for the quick response!

I apologize, I can't share the code exactly because it's private for work, and I'm not sure if a minimal example would exactly recreate it. I can however describe more of the setup (and specifically what might be weird):

  • in our usecase, we have recipe A. This recipe always get exported first.
  • Within recipe A, we have two kinds of nested class definitions that we use in other recipes (B, C, etc.) via python_requires. As far as I can tell, the generate methods in those nested class definitions do work in the child recipes that declare their dependency on them via python_requires.
  • I think the weird part about this issue is that the problematic generate call isn't in either of those nested class definitions, but in the base class recipe A defines.

Based on that last part... I am assuming because no recipe explicitly depends on A, but instead the nested classes within them, that A's generate() method doesn't get invoked ? Does that sound like something that would have worked on Conan1 but not Conan2?

Please let me know if there's anything more I can explain.

@memsharded
Copy link
Member

Based on that last part... I am assuming because no recipe explicitly depends on A, but instead the nested classes within them, that A's generate() method doesn't get invoked ? Does that sound like something that would have worked on Conan1 but not Conan2?

No, this python_requires hasn't changed that much in Conan 2, it should be pretty similar.

Could you please outline the A, B, C classes, how it looks like regarding the inheritance, is it using python_requires_extend?
Some skeleton recipes, without much code, just the classes and generate() methods to illustrate, will help.

@valenotary
Copy link
Author

Okay, I'll try my best. Also you were right, my apologies: We call python_requires on the actual recipe, but python_requires_extend on the nested classes within RecipeAConanFile.

# relevant conan and os imports 
# ... 

class RecipeAConanFile(ConanFile):
   name = "RecipeA"
   # ... other recipe attributes
  
   def generate(self):
      # print statements and other copying SHOULD be happening here, but doesnt 

   # ... other methods like requirements, init, etc

   class NestedModule(object):
      # other typical conanfile attributes and methods. we call python_requires to THIS nested class
      # generate is also defined in this base nested class, and gets invoked properly during conan command

And an example of a child recipe that would depend on stuff from recipe A (specifically, NestedModule):

# relevant conan and os imports 
# ... 

class RecipeBConanFile(ConanFile):
   python_requires = "RecipeA"
   python_requires_extend = "RecipeA.NestedModule"

So recipes B, C, ... etc. will basically have the first two members (python_requires, python_requires_extends) be the same.

@memsharded
Copy link
Member

Thanks for the feedback.
There is something unexpected there. The class to inherit from, class NestedModule(object):, shouldn't be nested in the class RecipeAConanFile(ConanFile).

Please check the examples in: https://docs.conan.io/2/reference/extensions/python_requires.html#extending-base-classes, see how MyBase is not a nested module. This hasn't change from Conan 1 (https://docs.conan.io/en/1.66/extending/python_requires.html#extending-base-classes).

I am not even sure how it would work, the following test fails:

    def test_reuse_nested(self):
        client = TestClient(light=True)
        tool = textwrap.dedent("""
            from conan import ConanFile
            class MyConanfileBase(ConanFile):
                name = "pkg"
                version = "0.1"
                class Nested:
                    def source(self):
                        self.output.info("My cool source!")
            """)
        reuse = textwrap.dedent("""
           from conan import ConanFile
           class PkgTest(ConanFile):
               python_requires = "pkg/0.1"
               python_requires_extend = "pkg.Nested"
           """)
        client.save({"tool/conanfile.py": tool,
                     "consumer/conanfile.py": reuse})
        client.run("create tool")
        client.run("source consumer")

with

AttributeError: module '2a5949fa-e39e-11ef-9cb6-f4a475d7794e' has no attribute 'Nested'
E           
E           ERROR: module '2a5949fa-e39e-11ef-9cb6-f4a475d7794e' has no attribute 'Nested'

@valenotary
Copy link
Author

Oh my bad! I completely misread the indents in the recipe file. It looks like RecipeA.py does have multiple non-nested classes in them. So it actually looks more like:

# imports 

class RecipeA(ConanFile):
   # stuff...

class ModuleA(object):
   # this is still python_requires_extended on child recipes

You are correct, the python_requires_extended classes were not nested. But now I'm even more confused as to how this wouldve worked before... We export the whole recipe, but we actually dont need that first RecipeA class at all seemingly?

@memsharded
Copy link
Member

Thanks for the feedback. So if the Nested class is put at the file scope, it seems the generate() method is inherited and used correctly:

def test_reuse_nested():
    c = TestClient(light=True)
    tool = textwrap.dedent("""
        from conan import ConanFile

        class MyConanfileBase(ConanFile):
            name = "pkg"
            version = "0.1"

        class Nested:
            def generate(self):
                self.output.info("Generate inherited!!")
        """)
    reuse = textwrap.dedent("""
       from conan import ConanFile
       class PkgTest(ConanFile):
           python_requires = "pkg/0.1"
           python_requires_extend = "pkg.Nested"
       """)
    c.save({"tool/conanfile.py": tool,
            "consumer/conanfile.py": reuse})
    c.run("create tool")
    c.run("install consumer")
    assert "Generate inherited!!" in c.out

So there might be some other thing, but I don't know what it could be, there might be some detail in your files that we are missing.

@valenotary
Copy link
Author

valenotary commented Feb 6, 2025

So inside MyConanfileBase as per your example there is also a generate method. And before the migration naively that generate method inside MyConanfileBase was just an import method that is now deprecated. It is that specific generate method inside MyConanfileBase that I'm wondering it isn't being invoked at all. I agree that the nested class's generate method should be called and I do observe that working as normal.

Also, my child recipes call python_requires on MyConanfileBase's name but also call python_requires_extend on the Nested class

@memsharded
Copy link
Member

There are different things there, the first point:

def test_reuse_nested():
    c = TestClient(light=True)
    tool = textwrap.dedent("""
        from conan import ConanFile

        class MyConanfileBase(ConanFile):
            name = "pkg"
            version = "0.1"

            def generate(self):
                raise Exception("This is a python requrires, it doesn't generate when used!!")

        class Nested:
            def generate(self):
                self.output.info("Generate inherited!!")
        """)
    reuse = textwrap.dedent("""
       from conan import ConanFile
       class PkgTest(ConanFile):
           python_requires = "pkg/0.1"
           python_requires_extend = "pkg.Nested"
       """)
    c.save({"tool/conanfile.py": tool,
            "consumer/conanfile.py": reuse})
    c.run("export tool")  # If you use create, it will raise the generate
    c.run("install consumer")  # this doesn't raise!
    assert "Generate inherited!!" in c.out

This test still works. You can see the generate() method of MyConanfileBase is not called or inherited when it is used, only the Nested class one, as expected.

Also, my child recipes call python_requires on MyConanfileBase's name but also call python_requires_extend on the Nested class

You mean you are inheriting both from MyConanfileBase and Nested? Inheriting from both is not recommended, and in general, inheriting from MyConanfileBase is not recommended either, but inheriting only from classes like Nested that do not extend ConanFile already.

Please let me know if that helps, or what else could we be missing.

@valenotary
Copy link
Author

Hmmm okay. I think maybe then we weren't using this correctly even in Conan 1 -- I'm gonna assume that the case where MyConanFileBase's generate/import method was expected to be called was incorrect in our implementation. And if you say that we should be using one or the other (and more correctly just using python_requires_extended on the Nested class then I can refactor our implementation to just keep that, remove the python_requires on MyConanFileBase and observe if everything still works.

Just out of curiousity, is it just undefined if we inherit from both, or just rendundant, or something else?

@memsharded
Copy link
Member

Just out of curiousity, is it just undefined if we inherit from both, or just rendundant, or something else?

yes, the main problem is that the python_requires_extend is not really regular Python inheritance. Is a bit of a hack, because it cannot be regular inheritance, because the class to inherit from doesn't even exist in disk and has to be downloaded at import time. So the extended class is injected as base class dynamically later, but that doesn't respect the same way the regular Python inheritance for multiple inheritance. It can behave a bit messy, and depend on things like the Python module loading order (as the python-requires are loaded only once, even if they are used in multiple packages).

So even if it might work in some cases, it is strongly discouraged, see https://docs.conan.io/2/knowledge/guidelines.html

Keep python_requires as simple as possible. Avoid transitive python_requires, keep them as reduced as possible, and at most, require them explicitly in a “flat” structure, without python_requires requiring other python_requires. Avoid inheritance (via python_requires_extend) if not strictly necessary, and avoid multiple inheritance at all costs, as it is extremely complicated, and it does not work the same as the built-in Python one.

@memsharded
Copy link
Member

Hi @valenotary

Any further question here? If not maybe the ticket can be closed? You can always open new tickets for any new issues. Thanks for your feedback.

@memsharded memsharded added type: question responded Responded by Conan team labels Feb 12, 2025
@valenotary
Copy link
Author

No that will be all thank you, feel free to close. I'll audit our usages of python_requires and try to keep it simple as the docs suggest.

@memsharded
Copy link
Member

Great, thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
responded Responded by Conan team type: question
Projects
None yet
Development

No branches or pull requests

2 participants