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

Feature: link editable packages #4181

Merged
merged 148 commits into from Jan 10, 2019

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Dec 20, 2018

Changelog: Feature: Add experimental support for packages in editable mode
Docs: conan-io/docs#1009

Substitutes #4152

Copy link
Contributor Author

@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.

A couple of comments

conans/client/command.py Outdated Show resolved Hide resolved
conans/client/installer.py Outdated Show resolved Hide resolved
conans/model/editable_cpp_info.py Show resolved Hide resolved
conans/test/functional/editable/commands/inspect_test.py Outdated Show resolved Hide resolved
@jgsogo
Copy link
Contributor Author

jgsogo commented Jan 8, 2019

There are still some open issues, all of them listed as checkboxes in the first comment. My opinion on them:


  • require_namespace, is this the name for the argument? it will raise when reading the file if some namespaces are found but not expected?

Note.- We haven't agree if this information at the cache level will be in a single file, inside a profile,... but we are taking it for granted.

Paths in the cache-file can be applied to an specific package or to every one (more on this in the next issue), while paths in the repo-file can be applied only to the package where the file is. So, the repo-file wont have namespaces (sections like [pck_name:includedirs]) while the repo-file needs to know which package the section is referring to.

The question is: do we raise if we found a section with namespaces in the repo-file? do we raise if we found a section without namespaces in the cache-file?

My opinion: if it is a dedicated file I will raise.


  • do we want to allow pattern matching in the .ini file?

In the cache-file, sections must be identified so we know which package they are referring to. At this stage of development, there are only two options:

  • [*:includedirs]: applies to every package (lower priority)
  • [pck_A:includedirs]: applies only to package pck_A (higher priority)

If we introduce some kind of package matching then sections like [pck_*:includedirs] will appear and we will need to agree on priority between different patterns (and we sure that the parser preserves this ordering)


  • do we need full conan-references in the .ini file?

In the cache-file, now we are considering just the package name, does it make sense to include full conan reference?


  • add an argument to the conan link command to allow setting a custom file with the information of paths (it will allow the user to skip the local repo file if it exists)

I would vote to merge this feature without that argument in the CLI, but I agree that it is the only way for the user to skip the local repo file if it exists in order to use the cache-file.


  • File naming: decide naming for CONAN_PACKAGE_LAYOUT_FILE and EDITABLES_FILE; both, the variable name and the underlying file.

I will go for CONAN_PACKAGE_LAYOUT_FILE (.conan_package_layout) and LINKED_PACKAGES_LAYOUT_FILE (linked_packages_layout.ini).

Also, I would change LINKED_FOLDER_SENTINEL to LINKED_PACKAGE_SENTINEL


I think that the best way to do it is: build the absolute path to the conanfile.py in the Command class (it knows about the current working dir) and the conan_api will receive it.


Needs @lasote feedback.

@@ -71,6 +72,27 @@ def __call__(self, parser, namespace, values, option_string=None):
setattr(namespace, self.dest, values)


class ConanFilePath(argparse.Action):
Copy link
Contributor

Choose a reason for hiding this comment

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

For me, the more aligned with the last comments would be to create a function in the conan API class or a function in the module while we define better how we want it. So, the command module can call that function and then pass the conanfile to the conan API method. I agree with @memsharded that the function to get the conanfile path doesn't belong to the command module.

conans/client/client_cache.py Show resolved Hide resolved
status = RECIPE_EDITABLE
# TODO: log_recipe_got_from_editable(reference)
# TODO: recorder.recipe_fetched_as_editable(reference)
metadata = self._client_cache.load_metadata(conan_reference)
Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is not exactly with the revisions, assign there a revision fix it by chance. In the info_on_child_test there is a node in the graph that is the "conanfile.py", that node has None/None@None/None reference. So, comparing with this code in the graph.py crashes because it doesn't early return:

    def __cmp__(self, other):
        if other is None:
            return -1
        elif self.conan_ref is None:
            return 0 if other.conan_ref is None else -1
        elif other.conan_ref is None:
            return 1

        if self.conan_ref == other.conan_ref:
            return 0

        # Cannot compare None with str
        if self.conan_ref.revision is None and other.conan_ref.revision is not None:
            return 1

        if self.conan_ref.revision is not None and other.conan_ref.revision is None:
            return -1

        if self.conan_ref < other.conan_ref:
            return -1
        return 1

Said that, @memsharded @jgsogo confirm that the None/None@None/None reference is intended, I assume that a better comparison code should be introduce for that case. Probably a unit test case would be necessary.



class EditableCppInfo(object):
WILDCARD = "*"
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm ok if at this point we only allow "*"

@@ -28,6 +29,7 @@
REGISTRY_JSON = "registry.json"
PROFILES_FOLDER = "profiles"
HOOKS_FOLDER = "hooks"
EDITABLES_FILE = "editable_packages.ini"
Copy link
Contributor

Choose a reason for hiding this comment

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

About the "general" file to declare layouts. I would like to discuss together the Jerry idea about having profiles for that.
As he said, from next release profile can be composable, that is, specify several profiles at the same time.
So, what if I declare a profile only defining editables stuff? I could have any number of profiles for any reason, not only an editables_packages.ini, like profiles/visual_old_projects.layout or profiles/linux_common_editables or whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If this is possible and those files/profile will contain sections listing editable_packages directories then we need to decide between two behaviors:

  • the last section found with the same name overrides the previous ones
  • each section will accumulate all the paths listed on every file.

This affects also to the criteria followed to get paths from the editables_packages.ini file (if we allow pattern matching we can come across several sections suitable for the same package)

conans/model/editable_cpp_info.py Outdated Show resolved Hide resolved
conans/model/editable_cpp_info.py Show resolved Hide resolved
conans/model/editable_cpp_info.py Outdated Show resolved Hide resolved
conans/paths/package_layouts/package_editable_layout.py Outdated Show resolved Hide resolved
@jgsogo
Copy link
Contributor Author

jgsogo commented Jan 9, 2019

There are just a couple of open questions yet, please one last effort and we can start thinking about merging it 💪

@ghost ghost assigned memsharded Jan 10, 2019
@memsharded memsharded merged commit be6121d into conan-io:develop Jan 10, 2019
@ghost ghost removed the stage: review label Jan 10, 2019
@jgsogo jgsogo deleted the feature/link-editable-packages branch January 10, 2019 15:54
NoWiseMan pushed a commit to NoWiseMan/conan that referenced this pull request Jan 11, 2019
* first try

* fix includes

* minor changes

* more import fixes

* fix more imports

* short path as decorator

* move SimplePaths to conans/paths/simple_paths.py

* move back constants to conans.paths

* moving folders to conans.paths

* remove diffs caused just by ordering of imports

* remove diffs caused just by ordering of imports

* no diffs

* no diff

* move is_case_insensitive_os to conans.paths

* add rm_conandir to windows imports

* move package_layouts to paths

* fix imports

* move LINKED_FOLDER_SENTINEL to conans.paths.__init__.py

* move 'get_package_layout' to 'SimplePaths'

* get rid of PackageBaseLayout, go for duck typing

* add parser for the file that will contain paths to includes, res, bins, libs,...

* rename PackageUserLayout > PackageEditableLayout

* interface for installed_as_editable info

* test with a header only editable package

* working example for header only lib

* add test for a library with settings and options (still header only)

* fix py2 test (declare string in source as unicode)

* change import to avoid conans.tools one, so we need to mock a different function

* make it compatible for win/linux

* create the editable file sentinel using --editable argument in install command

* add needed functions to simple_paths

* add helper functions to simple paths

* add package_metadata to new layouts

* add missing named var (something went wrong merging from develop)

* remove lines, not anymore there

* add package metadata to users one

* handle error: package folders needs a ConanFileReference

* reorder imports

* add missing import

* reorder imports

* if an editable package is dirty, log an error

* moved parser to conan_file

* move parsing for conan_package_layout closer to the ConanFile, we will be able to change it in the future

* placeholders should be preprended with settings|options

* fix typos, remove unused code

* fix bug/typo

* order imports

* remove unreachable code

* remove unreachable code

* remove comments

* move unittests to its folder

* install as editable (requires the package to be already in the cache)

* declare tests

* refactor the editable hack: rely on member attribute

* move imports

* rename file to match class name

* notify user about editable condition

* gather logic together

* make it editable from the very beginning, we don't care if it is editable or not

* any error will require us to delete the editable flag

* add tests

* add tests related to editable package removal

* rename tests

* add test for other commands

* add tests for conan commands

* Update conans/test/command/editable_reference/create_editable_package_test.py

Co-Authored-By: jgsogo <jgsogo@gmail.com>

* add changes from review

* move tests to final location

* remove unused var

* change variable name (it was a py conanfile)

* don0t care about the output, but check something

* remove comments

* add tests installing an editable package (and installing its dependencies)

* remove unused import

* move cpp_info overrides to an standalone model

* minor changes related to reviews

* mode create/remove editables to client_cache class

* renamed test folder

* remove code related to 'install --editable'

* link/unlink editable packages

* consider these tests

* handle remove command on editable

* adapt message

* removal is on its own test

* check after unlink

* add tests using linked packages

* add tests for several commands

* fix test related to utf8 and ordering

* apply ordering

* remove prints

* putting path between quotes seems to solve windows issue

* review (add more checks in tests)

* function changed its name

* no import in __init__.py

* remove installed as editable

* order is not guaranteed on info command output

* no need for os.path.join

* move EDITABLE before locking package layout

* no different behaviour for update

* new model for editable_cpp_info

* check that cache file has more priority than the repo one

* review

* remove test, it is accepted behaviour now

* review

* revert change, this line is needed to pass tests in Windows

* weird substitution is not needed, but path normalization is

* inform about link removal

* use conditional 'strict' arg when reusing cache for several TestClients

* pass string to conan_api, build ConanFileReference inside it

* add tests case for editable package without files, it will use package_info data

* remove unused import

* remove uneeded command

* get the path to the conanfile.py from the argument to 'link' command inside the 'conan_api'

* remove 'EditableCppInfo.create', rename loader ones to 'load(file...)' and 'loads(teext...)'

* rename func argument

* move CONAN_PACKAGE_LAYOUT_FILE to conans/paths

* reorder variables inside file

* renamed variables

* pep8

* not using revisions in editable nodes, fix ordering issue

* rename variables
NoWiseMan pushed a commit to NoWiseMan/conan that referenced this pull request Jan 11, 2019
* first try

* fix includes

* minor changes

* more import fixes

* fix more imports

* short path as decorator

* move SimplePaths to conans/paths/simple_paths.py

* move back constants to conans.paths

* moving folders to conans.paths

* remove diffs caused just by ordering of imports

* remove diffs caused just by ordering of imports

* no diffs

* no diff

* move is_case_insensitive_os to conans.paths

* add rm_conandir to windows imports

* move package_layouts to paths

* fix imports

* move LINKED_FOLDER_SENTINEL to conans.paths.__init__.py

* move 'get_package_layout' to 'SimplePaths'

* get rid of PackageBaseLayout, go for duck typing

* add parser for the file that will contain paths to includes, res, bins, libs,...

* rename PackageUserLayout > PackageEditableLayout

* interface for installed_as_editable info

* test with a header only editable package

* working example for header only lib

* add test for a library with settings and options (still header only)

* fix py2 test (declare string in source as unicode)

* change import to avoid conans.tools one, so we need to mock a different function

* make it compatible for win/linux

* create the editable file sentinel using --editable argument in install command

* add needed functions to simple_paths

* add helper functions to simple paths

* add package_metadata to new layouts

* add missing named var (something went wrong merging from develop)

* remove lines, not anymore there

* add package metadata to users one

* handle error: package folders needs a ConanFileReference

* reorder imports

* add missing import

* reorder imports

* if an editable package is dirty, log an error

* moved parser to conan_file

* move parsing for conan_package_layout closer to the ConanFile, we will be able to change it in the future

* placeholders should be preprended with settings|options

* fix typos, remove unused code

* fix bug/typo

* order imports

* remove unreachable code

* remove unreachable code

* remove comments

* move unittests to its folder

* install as editable (requires the package to be already in the cache)

* declare tests

* refactor the editable hack: rely on member attribute

* move imports

* rename file to match class name

* notify user about editable condition

* gather logic together

* make it editable from the very beginning, we don't care if it is editable or not

* any error will require us to delete the editable flag

* add tests

* add tests related to editable package removal

* rename tests

* add test for other commands

* add tests for conan commands

* Update conans/test/command/editable_reference/create_editable_package_test.py

Co-Authored-By: jgsogo <jgsogo@gmail.com>

* add changes from review

* move tests to final location

* remove unused var

* change variable name (it was a py conanfile)

* don0t care about the output, but check something

* remove comments

* add tests installing an editable package (and installing its dependencies)

* remove unused import

* move cpp_info overrides to an standalone model

* minor changes related to reviews

* mode create/remove editables to client_cache class

* renamed test folder

* remove code related to 'install --editable'

* link/unlink editable packages

* consider these tests

* handle remove command on editable

* adapt message

* removal is on its own test

* check after unlink

* add tests using linked packages

* add tests for several commands

* fix test related to utf8 and ordering

* apply ordering

* remove prints

* putting path between quotes seems to solve windows issue

* review (add more checks in tests)

* function changed its name

* no import in __init__.py

* remove installed as editable

* order is not guaranteed on info command output

* no need for os.path.join

* move EDITABLE before locking package layout

* no different behaviour for update

* new model for editable_cpp_info

* check that cache file has more priority than the repo one

* review

* remove test, it is accepted behaviour now

* review

* revert change, this line is needed to pass tests in Windows

* weird substitution is not needed, but path normalization is

* inform about link removal

* use conditional 'strict' arg when reusing cache for several TestClients

* pass string to conan_api, build ConanFileReference inside it

* add tests case for editable package without files, it will use package_info data

* remove unused import

* remove uneeded command

* get the path to the conanfile.py from the argument to 'link' command inside the 'conan_api'

* remove 'EditableCppInfo.create', rename loader ones to 'load(file...)' and 'loads(teext...)'

* rename func argument

* move CONAN_PACKAGE_LAYOUT_FILE to conans/paths

* reorder variables inside file

* renamed variables

* pep8

* not using revisions in editable nodes, fix ordering issue

* rename variables
@lasote
Copy link
Contributor

lasote commented Jan 17, 2019

@jgsogo the docs are not linked in the body.

@danimtb
Copy link
Member

danimtb commented Jan 17, 2019

Added docs! 💪

lasote pushed a commit that referenced this pull request Feb 26, 2019
* add new arch [armv5el, armv5hf]

- armv5el -> armv5 soft-float
- armv5hf -> armv5 hard-float

There are still many devices based on these architectures especially embedded devices like Danelec devices, Synology etc...

*  support for armv5el, armv5hf

- made changes to b2 generator
- add tests

* Feature/deterministic short paths (#4238)

* Add sha256 helper function

* Implement hashed_redirect

Generate deterministic short paths. This makes it possible to re-use the
compilation cache database after the re-installation of conan packages.

Resolves #3971

* Write the full path to the Conan cache in the shortened directory.

Resolves #3971

* Adds new architectures: ppc32, armv8_32 and armv8.3 (#4195)

* new archs in settings.yaml

* Added new archs to migration

* Update migration to original settings.yml

* Updated b2 generator with new archs

* pure review of b2 generator

* Added new archs to get_gnu_triplet()

* revert changes on b2 generator

* review

* Back to aaarch64 and link for ilp32 clarification

* Conversion to apple arch

* added armv8.3 to get_gnu_triplet()

* feature/enabling environment variables unset action (#4224)

* Enabling the possibility of unsetting variables by means of appending such variable with value equal to None. Tests covering tools.env.environment_append have been included

* Fixing PEP8 line width requirement

* Fixing PEP8 line width requirement

* Fixing path separator used in the unit test considering its os-dependant value

* Fixing how the env_vars dictionary is being modified. Instead of removing values on-the-fly, It is previourly checked which should be removed by iterating on the keys.

* Fixing how the env_vars dictionary is being modified again. Iterating on keys didn't work enough and now it is decoupled the iteration by using the keys that should be removed and were previously stored

* Fix. Setting all the variables to None (unsetting all of them) effectively removes them from environment.

- Applying changes needed.
- Adding regression test.
- Adding additional test covering the case where no variables are asked to be removed (empty dict).
- Improve tests naming.

* Removed versions (#4267)

* Fixed bug with cached default profile (#4256)

* Fixed bug

* Docstring

* removed caching in client_cache of settings and profile

* Feature: link editable packages (#4181)

* first try

* fix includes

* minor changes

* more import fixes

* fix more imports

* short path as decorator

* move SimplePaths to conans/paths/simple_paths.py

* move back constants to conans.paths

* moving folders to conans.paths

* remove diffs caused just by ordering of imports

* remove diffs caused just by ordering of imports

* no diffs

* no diff

* move is_case_insensitive_os to conans.paths

* add rm_conandir to windows imports

* move package_layouts to paths

* fix imports

* move LINKED_FOLDER_SENTINEL to conans.paths.__init__.py

* move 'get_package_layout' to 'SimplePaths'

* get rid of PackageBaseLayout, go for duck typing

* add parser for the file that will contain paths to includes, res, bins, libs,...

* rename PackageUserLayout > PackageEditableLayout

* interface for installed_as_editable info

* test with a header only editable package

* working example for header only lib

* add test for a library with settings and options (still header only)

* fix py2 test (declare string in source as unicode)

* change import to avoid conans.tools one, so we need to mock a different function

* make it compatible for win/linux

* create the editable file sentinel using --editable argument in install command

* add needed functions to simple_paths

* add helper functions to simple paths

* add package_metadata to new layouts

* add missing named var (something went wrong merging from develop)

* remove lines, not anymore there

* add package metadata to users one

* handle error: package folders needs a ConanFileReference

* reorder imports

* add missing import

* reorder imports

* if an editable package is dirty, log an error

* moved parser to conan_file

* move parsing for conan_package_layout closer to the ConanFile, we will be able to change it in the future

* placeholders should be preprended with settings|options

* fix typos, remove unused code

* fix bug/typo

* order imports

* remove unreachable code

* remove unreachable code

* remove comments

* move unittests to its folder

* install as editable (requires the package to be already in the cache)

* declare tests

* refactor the editable hack: rely on member attribute

* move imports

* rename file to match class name

* notify user about editable condition

* gather logic together

* make it editable from the very beginning, we don't care if it is editable or not

* any error will require us to delete the editable flag

* add tests

* add tests related to editable package removal

* rename tests

* add test for other commands

* add tests for conan commands

* Update conans/test/command/editable_reference/create_editable_package_test.py

Co-Authored-By: jgsogo <jgsogo@gmail.com>

* add changes from review

* move tests to final location

* remove unused var

* change variable name (it was a py conanfile)

* don0t care about the output, but check something

* remove comments

* add tests installing an editable package (and installing its dependencies)

* remove unused import

* move cpp_info overrides to an standalone model

* minor changes related to reviews

* mode create/remove editables to client_cache class

* renamed test folder

* remove code related to 'install --editable'

* link/unlink editable packages

* consider these tests

* handle remove command on editable

* adapt message

* removal is on its own test

* check after unlink

* add tests using linked packages

* add tests for several commands

* fix test related to utf8 and ordering

* apply ordering

* remove prints

* putting path between quotes seems to solve windows issue

* review (add more checks in tests)

* function changed its name

* no import in __init__.py

* remove installed as editable

* order is not guaranteed on info command output

* no need for os.path.join

* move EDITABLE before locking package layout

* no different behaviour for update

* new model for editable_cpp_info

* check that cache file has more priority than the repo one

* review

* remove test, it is accepted behaviour now

* review

* revert change, this line is needed to pass tests in Windows

* weird substitution is not needed, but path normalization is

* inform about link removal

* use conditional 'strict' arg when reusing cache for several TestClients

* pass string to conan_api, build ConanFileReference inside it

* add tests case for editable package without files, it will use package_info data

* remove unused import

* remove uneeded command

* get the path to the conanfile.py from the argument to 'link' command inside the 'conan_api'

* remove 'EditableCppInfo.create', rename loader ones to 'load(file...)' and 'loads(teext...)'

* rename func argument

* move CONAN_PACKAGE_LAYOUT_FILE to conans/paths

* reorder variables inside file

* renamed variables

* pep8

* not using revisions in editable nodes, fix ordering issue

* rename variables

* fixing version ranges with spaces (#4273)

* fixing version ranges with spaces

* fix broken test

* simplified testing

* more error cases

* [WIP] Rename common variables to conventional ones (#4272)

* rename internal member 'PackageReference.conan' to 'PackageReference.ref'

* rename internal member 'PackageReference.conan' to 'PackageReference.ref'

* working on ConanFileRefs

* working on ConanFileRefs

* working on PackageReference

* working on ClientCache

* back to ConanFileReference

* rename member 'Node::conan_ref'

* rename member 'PythonRequires::conan_ref'

* renaming vars in comments, docs,...

* lost references

* remove duplicated uneeded conversion to str

* some loose fringes

* minor fix, review by @lasote #4272

* rename PackageReference::package_id' to 'PackageReference::id'

* loose fringes

*  support for armv5el, armv5hf

- made changes to b2 generator
- add tests

* add handling for arm5hf to get_gnu_triplet
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants