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

import conanfile just once #4095

Merged

Conversation

memsharded
Copy link
Member

@memsharded memsharded commented Dec 10, 2018

Changelog: Feature: Optimize the graph loading, specially of build-requires, by avoiding importing more than once the same conanfile.py
Docs: Omit

This optimization could be relevant to flatten the dependency graph when build requires are applied to the whole graph.

@ghost ghost assigned memsharded Dec 10, 2018
@ghost ghost added the stage: review label Dec 10, 2018
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make a couple of changes, but not so important.

conans/test/model/transitive_reqs_test.py Outdated Show resolved Hide resolved
conans/client/loader.py Outdated Show resolved Hide resolved
client = TestClient()
conanfile = """from conans import ConanFile
mycounter = 0
class Pkg(ConanFile):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, add a counter associated with the instance.

conans/test/optimize_conanfile_load_test.py Outdated Show resolved Hide resolved
@@ -181,6 +181,7 @@ class SayConan(ConanFile):

def root(self, content, update=False):
processed_profile = ProcessedProfile()
self.loader.cached_conanfiles = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as @jgsogo. It shouldn't be necessary. Why did you add it? Maybe we are missing something.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the above comment.

Copy link
Contributor

@lasote lasote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good but take a look to our comments.

@lasote
Copy link
Contributor

lasote commented Jan 2, 2019

Missing docs! Shouldn't be merged until we have it.

@lasote lasote added the whiteboard Put in common label Jan 2, 2019
@lasote
Copy link
Contributor

lasote commented Jan 2, 2019

As you wanted to discuss it I tagged it as whiteboard so you can explain it.

@memsharded memsharded removed the whiteboard Put in common label Jan 3, 2019
@memsharded memsharded assigned lasote and unassigned memsharded Jan 3, 2019
@memsharded memsharded added this to the 1.12 milestone Jan 3, 2019
@memsharded memsharded assigned memsharded and unassigned lasote Jan 3, 2019
@memsharded memsharded merged commit 643a9c8 into conan-io:develop Jan 3, 2019
@ghost ghost removed the stage: review label Jan 3, 2019
@memsharded memsharded deleted the feature/import_conanfile_once branch January 3, 2019 19:29
NoWiseMan pushed a commit to NoWiseMan/conan that referenced this pull request Jan 9, 2019
* import conanfile just once

* reset cache

* fixing tests

* review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants