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

use of python_requires on editable package fails #5358

Closed
3 tasks done
alexandre-perrin opened this issue Jun 15, 2019 · 5 comments
Closed
3 tasks done

use of python_requires on editable package fails #5358

alexandre-perrin opened this issue Jun 15, 2019 · 5 comments

Comments

@alexandre-perrin
Copy link

alexandre-perrin commented Jun 15, 2019

Environment

  • python 3.6
  • Conan version: 1.16.0
  • OS: Windows 10

Description

When using a common dependency python_requires package as editable, the PackageEditbleLayout raise the following exception:

Operation not allowed on a package installed as editable

Reproduce

The following can be used for reproducing the error:

Packages:

  • common
    • conanfile.py
  • userproject
    • conanfile.py

Common package conanfile.py:

from conans import ConanFile

class CommonConanFile(ConanFile):
    name = "common"
    version = "1.0"

User package conanfile.py:

from conans import ConanFile, python_requires

base = python_requires('common/1.0@user/channel')

class Conan(ConanFile):
    name = "userproject"
    version = "1.0"

Reproduce the issue:

> conan editable add .\common common/1.0@user/channel
Reference 'common/1.0@user/channel' in editable mode
> conan editable add .\userproject userproject/1.0@user/channel
ERROR: Error loading conanfile at '~\userproject\conanfile.py': Operation not allowed on a package installed as editable

  • I've read the CONTRIBUTING guide.
  • I've specified the Conan version, operating system version and any tool that can be relevant.
  • I've explained the steps to reproduce the error or the motivation/use case of the question/suggestion.
@alexandre-perrin
Copy link
Author

alexandre-perrin commented Jun 15, 2019

This fails because it tries to call export and export_sources as an editable package, when actually nothing requires to be exported.
I suggest a simple fix such as detecting if the package_layout object is weither editable or not.

patch of client/graph/python_requires.py:

index 2c20e6ec..3e64898a 100644
--- a/conans/client/graph/python_requires.py
+++ b/conans/client/graph/python_requires.py
@@ -60,8 +60,12 @@ class ConanPythonRequire(object):
                 python_require = self._look_for_require(conanfile.alias)
             else:
                 package_layout = self._proxy._cache.package_layout(new_ref, conanfile.short_paths)
-                exports_sources_folder = package_layout.export_sources()
-                exports_folder = package_layout.export()
+                if hasattr(package_layout, 'editable_cpp_info'):
+                    # Editable package does not require exporting
+                    exports_sources_folder = exports_folder = None
+                else:
+                    exports_sources_folder = package_layout.export_sources()
+                    exports_folder = package_layout.export()
                 python_require = PythonRequire(new_ref, module, conanfile,
                                                exports_folder, exports_sources_folder)
             self._cached_requires[require] = python_require

Would it be suitable ?

@lasote
Copy link
Contributor

lasote commented Jun 17, 2019

Hi, I don't like that fix especially, we will take a look in case we have a better fix for this.
Thanks for reporting!

@jgsogo jgsogo added this to To do in Packages in editable mode via automation Jun 18, 2019
@jgsogo jgsogo self-assigned this Jun 18, 2019
@jgsogo
Copy link
Contributor

jgsogo commented Jun 18, 2019

Hi! IMO this is more a new feature than a bugfix (and not sure if it is a feature we can provide right now). TL;DR: a python_requires as editable package is like something forbidden right now: editable requires settings and python_requires doesn't know about them.


To implement this feature we would need the following changes:

  • Layout files will need two new sections: [exports_sources_folder] and [exports_folder]

    • current implementation of python_requires, allows only one folder for export_sources and export folders, so these fields in the layout file should allow only one entry (like the source_folder and build_folder actually do).
  • We will need to add a few lines into the PackageEditableLayout class

    class PackageEditableLayout:
        def export(self):
            try:
                layout = self.editable_cpp_info()
                if layout:
                    return layout.<get-export-folder>(ref, settings, options)  # BIG PROBLEM!!!
            except ConanException:
                raise ConanException("Operation not allowed on a package installed as editable")
    
        def export_sources(self):
            # More or less the same as above

But there is a huge problem in order to implement this behavior: to parse the layout file we need the settings and options, but the moment we require the exports and exports_sources folder (the moment we are calling the python_requires(...) function) we know nothing about settings and options...


Before trying to implement something, we need to triage again this issue, and I cannot think about a nice way to implement it:

  1. Provide mocked settings and options objects to the layout file... 🙈
  2. Pass the settings and options to the point where the python_requires is retrieved/loaded and use them to parse the layout file... but by definition a python_requires should not depend on settings (nor options) as it is related to reusing sources, not binaries. If we follow this path, why not consuming binaries too?

Option (1) is not a solution, and option (2) would break the idea behind python_requires. 😢

@jgsogo jgsogo assigned lasote and unassigned jgsogo Jun 18, 2019
@lasote lasote removed the type: bug label Jun 19, 2019
@lasote lasote removed this from the 1.17 milestone Jun 19, 2019
birgerbr added a commit to Imerso3D/conan that referenced this issue Sep 17, 2019
@memsharded
Copy link
Member

Hi @alexandre-perrin

there has been progress in the python_requires, and we are proposing some changes to the way they are defined and used in: #5804

Some important difference:

  • to reuse files from python-requires, it is not possible to do it in exports_sources. It was just too error prone and fragile. To reuse assets from a python_require the only way will be to exports those assets, and explicitly using them with something like:
def build(self):
        sources = self.python_requires["tool"].path
        file_h = os.path.join(sources, "file.h")

There are tests in that PR that check that this way works both when python-requires are in the cache or they are editable (as long as the exports maintain the relative layout)

Doing this change was necessary to solve some major issues with python_requires. Now it will be possible to make python_requires to affect consumers packageIDs, thus forcing rebuilds, and they will play better with very important features like lockfiles.

birgerbr added a commit to Imerso3D/conan that referenced this issue May 26, 2020
birgerbr added a commit to Imerso3D/conan that referenced this issue May 26, 2020
birgerbr added a commit to Imerso3D/conan that referenced this issue Jan 29, 2021
birgerbr added a commit to Imerso3D/conan that referenced this issue Oct 1, 2021
birgerbr added a commit to Imerso3D/conan that referenced this issue Aug 23, 2022
birgerbr added a commit to Imerso3D/conan that referenced this issue Sep 16, 2022
@memsharded
Copy link
Member

python_requires support editable in 2.0 correctly, closing this issue as solved.

Packages in editable mode automation moved this from To do to Done Mar 18, 2023
birgerbr added a commit to Imerso3D/conan that referenced this issue Nov 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

4 participants