Skip to content
This repository has been archived by the owner on May 21, 2020. It is now read-only.

[RFC] Two recipes for regular flex and flex_installer in one repo: use python_requires #5

Closed
wants to merge 2 commits into from

Conversation

madebr
Copy link

@madebr madebr commented Jan 31, 2019

In response to bincrafters/community#625

This pr is very similar to #4 in that it creates multiple recipes in one repo.
This main difference is that this one is using python_requires.
This allows the common logic to be put in a common base class flex_base, which is subclassed by flex_installer and flex.

Imho, it looks much cleaner.
But alas, it has a major problem (and showstopper):

  • The python_requires in the flex and flex_installer recipe requires a full conan reference.
    In this pr, this reference is hardcoded to flex_base/2.6.4@bincrafters/stable which is not correct.
    The username and channel of flex_base should default to be the same as the username and channel of the flex_installer and flex recipe.

I do not know whether this is a limitation of python_requires or a limitation of my knowledge.
Imho, a ConanFile class should have the ability to override the username and channel of a python_requires.
Please comment if you know how to fix this.

Notes:

@jgsogo
Copy link
Member

jgsogo commented Feb 1, 2019

@madebr, about the python_requires issue I can point you to this comment conan-io/conan#4355 (comment), Conan needs to know which python_requires to use (flex_base) before trying to resolve the recipe (flex). The reason for this is that a python-requires might affect how the reference is computed and thus the pyreq should be fully resolved before, think about something like this:

base = python_requires("name/version@user/channel")

class MyLib(ConanFile):
    name = "mylib"
    version = base.compute_my_version()

how could I know MyLib::version without locking the base = python_req...?

@madebr
Copy link
Author

madebr commented Feb 1, 2019

@jgsogo Thanks for the tip. Good to know that an explicit version is required in derived conan classes.

My main problem/question remains though: the dependency on flex_base/2.6.4@bincrafters/stable is explicit. This will make it impossible to create a flex package for the testing channel without changing the string to flex_base/2.6.4@bincrafters/testing.

In other words, this is impossible:

conan export flex_base someuser/somechannel
conan create flex_installer someuser/somechannel
conan create flex someuser/somechannel

@jgsogo
Copy link
Member

jgsogo commented Feb 1, 2019

It cannot depend on the version of the recipe that is using the python requires, but you can do this without any problem:

username = os.getenv("CONAN_USERNAME")
channel = os.getenv("CONAN_CHANNEL")

base = python_requires("flex_base/2.6.4@{}/{}".format(username, channel))

class MyLib(base.ConanFile):
    name = "flex"
    ...

and those env variables can be populated from the command line or CI itself.

@madebr
Copy link
Author

madebr commented Feb 1, 2019

@jgsogo
The problem with that approach is that those variables are not defined when this package is used as a dependency.

If a package has a build_requires("flex_installer/2.6.4@bincrafters/stable"), an error will be thrown because flex_base/2.6.4@None/None is not found.
Using a default in get_env is also not appropriate.

Apart from that, I don't think it is good design to let environment variables define dependencies.
The dependency graph should be independent of environment variables.

@jgsogo
Copy link
Member

jgsogo commented Feb 1, 2019

Now I understand your problem, this is more related to this other issue (conan-io/conan#3988). I think we haven't come up with a solution yet 😞

@SSE4
Copy link
Member

SSE4 commented Apr 30, 2019

outdated by #7

@SSE4 SSE4 closed this Apr 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants