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

Implement recipe_folder attribute #7314

Merged
merged 4 commits into from Jul 22, 2020

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Jul 5, 2020

Changelog: Feature: Define recipe_folder attribute pointing to the folder containing conanfile.py
Docs: conan-io/docs#1785

Close #7312
Close #7315

Would help with:

@jgsogo the most important point of this PR is if we really want to allow this, or what Pandora box this could open, and allow too crazy things.

@gsemet
Copy link

gsemet commented Jul 6, 2020

Would be great :)

@jgsogo
Copy link
Member

jgsogo commented Jul 6, 2020

The conanfile.py can always retrieve its own path (os.path.dirname(__file__)), some functions already have access to this path, the hooks,... so probably this folder is something expected. Anyway, we need to write down what is available in each function and what value it takes, for example, I can never remember what is the value of conanfile_path in the export stage, the local path or the cache one?

Another alternative would be to remove always this folder from every function and rely on the conandata.yml. This file is exported together with the conanfile, it can store any value (values can be populated in the export stage too!) and the content is loaded by Conan in the attribute self.conan_data. Will this be a valid approach for your use-case, @gsemet?

@gsemet
Copy link

gsemet commented Jul 6, 2020

Hi, since I am working on a base class that will be used by python_requires, __file__ is not the final conanfile, so I would prefer having this information in self.recipe_folder. I am working on overriding the requirements hook method to allow users to write their dependencies in a txt file that will be exported aside of the conanfile.py. I do not know how conandata.yaml works, but as look as I can read the file content from the requirements method, that's fine for me.

I would prefer having this patch integrated in the next version, as of today, I do a search for the wanted file in the current folder and its parent (most of the time the conanfile will be in the parent of the current folder), but that's not a long term solution.

I will be describing my use case in another ticket to see if I am wrong somewhere

@sztomi
Copy link
Contributor

sztomi commented Jul 8, 2020

I have a use-case for this: accessing the conanfile in a generator. The actual context is a bit complicated but I'll try to explain in detail (so that you can get a sense if this is a use-case that you want to support in conan):

Our main project that consumes conan packages is Plex Media Server (PMS). The PMS build itself creates distro-agnostic tarballs that will be processed to create all kinds of packages: MSI installer for Windows, debian packages for debian etc. And since PMS itself is also used as a dependency in our clients, we also create a conan package from PMS at this stage. For that to work, we copy the conanfile at build time so it is present in these distro-agnostic tarballs.

We are working on decoupling conan usage from PMS. Instead of using conan install directly, we are using conan to generate a tarball for each configuration that will be consumed by PMS (when the dependencies are built, i.e. outside the PMS build). An alternative cmake buildinfo is written that points to the contents of this tarball. All this is accomplished in a custom generator.

But this also means that we no longer have a conanfile in the PMS repository, but we want to keep packaging PMS as a conan package. So we need a way to copy the conanfile into the tarball contents.

Right now, I'm copying the conanfile in the script that drives conan (i.e. it calls conan install with this custom generator). But it would be better if the generator was standalone and could really gather everything on its own. From a generator I can't use conanfile.__file__ or inspect.getfile because of the way the recipe class is imported and instantiated.

@gsemet
Copy link

gsemet commented Jul 12, 2020

Hi. as recommended by #7316, I switched to use conandata.yml since it is exported by defaut, and works great :)

@jgsogo
Copy link
Member

jgsogo commented Jul 17, 2020

Hi again!

I think that the use-vase that originated this PR is satisfied via conandata.yml and, given that we have some doubts about providing the recipe_folder, IMO we can close this one and probably related issues, is it ok, @gsemet?

@sztomi your use case is different as you want to access the conanfile.py file from a generator. If that generator is listed in the conanfile itself, it can be very challenging in the general case: you can run conan install <the-conanfile> -g <your-generator> locally, but at that moment the local conanfile might not be complete: the export stage might be creating the conandata.yml with the SCM info (or modifying the conanfile.py itself with that data)... I think generators are not prepared to use information from the recipe where they are declared.

@gsemet
Copy link

gsemet commented Jul 17, 2020

Indeed the use case is covered by the conandata and I especially like the fact it is automatically exported. So I do not need this feature anymore, but it would be great to still have this ability, on the principle. If it does not have unwanted side effects, of course.

In short, that's ok for me if this feature does not land in conan, for the moment

@jgsogo jgsogo assigned memsharded and unassigned jgsogo Jul 17, 2020
@memsharded
Copy link
Member Author

memsharded commented Jul 17, 2020

This issue reported here: #7355 would not be good enough with conandata.yml, as they use a central definition for different packages. As they are not using python_requires the os.path.dirname(__file__) works for them, but they will benefit from this recipe_folder attribute, just in case they start using python_requires.

@jgsogo
Copy link
Member

jgsogo commented Jul 17, 2020

I think it is totally fine to add this recipe_folder attribute, but as you have doubts my proposal was to postpone it as issues requesting it were satisfied with other features available. But, IMO, it is needed for completeness and to make some flows more robust. Also, as long as it is available in the hooks, it can safely be added to the recipe itself (or people will use the hook to set the value)

We only need to document which path it is (it is always the place where we read the recipe from, right?), we need to check that it is the same folder as the conanfile_path in the hooks, and check if there is any corner case with editable packages.

@memsharded
Copy link
Member Author

memsharded commented Jul 19, 2020

This feature is also requested (or at least would be very convenient) here: #7268

@memsharded memsharded merged commit 6f79e27 into conan-io:develop Jul 22, 2020
2 checks passed
@memsharded memsharded deleted the feature/recipe_folder branch Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants