implementing contains for option in self.info.options #7303

memsharded · 2020-07-02T22:44:15Z

Changelog: Fix: Implement missing __contains__ method, so checking if "myoption" in self.info.options is possible in package_id().
Docs: Omit

Fix #7299

jgsogo

The only documented action with self.info.options is to remove them: del self.info.options.shared (here), unless we are adding more actions to the documentation we shouldn't add tests that prevent us from changing what is not documented. Maybe it is the tool shared_library_package_id implementation the one that needs a patch.

About the check if not self.info.options.whatever: if we were talking about self.options there was a request to raise an error for not existing options (maybe @uilianries remembers and can provide a link), the idea behind that was to prevent typos. I would expect (for devs sanity) the same behavior here.

memsharded · 2020-07-03T11:20:32Z

The problem with this is reported in #7299, it is not something only internal to the shared_library_package_id. It is possible and valid Python syntax that someone checks if "something" in self.info.options, and at the moment this hangs the computer for a very long time until it crashes.

So I think this is not a feature, but a bugfix. Maybe we want to raise for that syntax and say it is not valid syntax, lets agree on the behavior, but the idiom if "shared" in self.options: is documented to work, so I think in this case it should probably work the same.

about the check if not self.info.options.whatever: if we were talking about self.options there was a request to raise an error for not existing options

This will possibly be too breaking for users relying on this behavior, I wouldn't change it to raise an error. It is currently undefined in the docs (at least I haven't found any reference), I don't think it could be declared a bug and raise if we hadn't do it so far.

jgsogo · 2020-07-03T12:04:36Z

Ok, let's make if "something" in self.info.options something valid, but write a unittest, do not use it in a ConanFile, it is not valid syntax in a ConanFile, we can change it if we want.

self.options.typo already raises, self.info.options.typo should raise too if we want the same behavior.

memsharded · 2020-07-14T12:34:53Z

conans/model/info.py

-        if self.full_options.shared:
+        if "shared" in self.full_options and self.full_options.shared:


I needed to change this, as now missing options also raise in self.info.options fields.

memsharded · 2020-07-14T12:35:54Z

conans/model/options.py

@@ -71,7 +72,7 @@ def __nonzero__(self):

    def __getattr__(self, attr):
        if attr not in self._dict:
-            return None
+            raise ConanException(option_not_exist_msg(attr, list(self._dict.keys())))


Important change behavior, to align with the raise of self.option.nonexisting, this is a bug fix.

Yes, recipes with typos will start to break, but I think this has been requested previously to avoid typos.

def package_id(self): del self.info.options.salkdjfsafldasj

maybe @uilianries remembers...

OK, lets go for it, and lets see if someone reports their builds breaking because of relying on this bug behavior.

memsharded · 2020-07-14T12:36:25Z

conans/test/functional/options/options_test.py

-                conanfile = conanfile_libB.replace(
-                    "        self.options[\"*\"].shared = self.options.shared", "")
-
+                conanfile = conanfile_libb.replace("self.options[", "#")


This was wrongly testing twice the same thing.

Yes, without changing anything else everything keeps working... I wonder what was the purpose of the line:

self.options["*"].shared = self.options.shared

It looks like a call like conan create . libB/0.1@danimtb/testing -o libB:shared=True is missing... to check that with and without that line the assignment for libA is different. Or maybe conan create . libB/0.1@danimtb/testing -o libB:shared=True -o *:shared=False

jgsogo

Why the test with conan create . Pkg/0.1@user/testing -o *:shared=True starts to fail?

conans/test/functional/options/options_test.py

jgsogo · 2020-07-16T15:10:32Z

conans/test/functional/options/options_test.py

-                conanfile = conanfile_libB.replace(
-                    "        self.options[\"*\"].shared = self.options.shared", "")
-
+                conanfile = conanfile_libb.replace("self.options[", "#")


Yes, without changing anything else everything keeps working... I wonder what was the purpose of the line:

self.options["*"].shared = self.options.shared

It looks like a call like conan create . libB/0.1@danimtb/testing -o libB:shared=True is missing... to check that with and without that line the assignment for libA is different. Or maybe conan create . libB/0.1@danimtb/testing -o libB:shared=True -o *:shared=False

conans/test/functional/package_id/package_id_test.py

jgsogo · 2020-07-16T15:22:19Z

conans/model/options.py

@@ -71,7 +72,7 @@ def __nonzero__(self):

    def __getattr__(self, attr):
        if attr not in self._dict:
-            return None
+            raise ConanException(option_not_exist_msg(attr, list(self._dict.keys())))


Yes, recipes with typos will start to break, but I think this has been requested previously to avoid typos.

def package_id(self): del self.info.options.salkdjfsafldasj

maybe @uilianries remembers...

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

) * implementing __contains__ for option in self.info.options * add test * more tests * clarified test * changed self.info.options.whatever to raise * improve tests * fix tests * Update conans/test/functional/package_id/package_id_test.py Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> * Update conans/test/functional/package_id/package_id_test.py Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> * fixed test Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

… user (#7390) * Only add User-Agent to headers dict if there was none provided by the user * [cpp_info] [refact] Do not assign to 'xxxx_paths' fields (#7276) * use get_configs instead of config * fixing tests * fix tests * fix tests * move pkg_config logic to cpp_info so everyone uses the same * 'name' from cpp_info needs the generator it is requested for * configs is a private dict now * revert change in init * add test for Conan v2 behavior * deps_cpp_info['deps'].name warns to use get_name instead * use cpp_info properly * compare as lists (convert possible iterables to lists) * set legit values in cpp_info * readonly fields expected to be iterable (not nessessaryly lists) * remove print statement from test * let any iterable type in * revert test changes * use property 'configs' to retrieve the different configs * check type for cpp_info/deps_cpp_info objects * just a list * Do not fail with 'conan remove -r remote -p' if there are no packages in the remote (#7338) * remove only if there are packages to remove * working on v1 too * no need to scape an r-string * v1 -> no revs * if not conaninfo, search doesn't work * propagate 404 only if there are packages for the reference * Update conans/client/rest/rest_client_v1.py Co-authored-by: James <james@conan.io> * when it is not none * package_ids is always a list. Capture unrelated errors Co-authored-by: James <james@conan.io> * [cpp_info] [refact] Rewrite text generator parser (#7277) * use get_configs instead of config * fixing tests * fix tests * fix tests * move pkg_config logic to cpp_info so everyone uses the same * 'name' from cpp_info needs the generator it is requested for * configs is a private dict now * revert change in init * add test for Conan v2 behavior * deps_cpp_info['deps'].name warns to use get_name instead * some attributes doesn't make sense at the base level * rewrite txt parser * revert test changes * use property 'configs' to retrieve the different configs * Avoid usage of '__len__' hook * fixing test: never root * read names from txt serialization * the only information that is serialized in the 'txt' generated file is the name for the txt generator * use ordereddict to retrieve deps in order * add test for next Conan v2 * remove commented lines * remove unused method * wrong behavior in py2 for it scope * fix tests in windows * revert single change * relax msbuild generator to not fail in Linux (#7361) * relax msbuild generator to not fail in Linux * fix test * cli 2.0: Add commands management (#7278) * add management for commands 2.0 * refactor cli * fail on empty docs * rename _conan to _conan_api * minimum required version * add fixme for conan factory * add discuss * raise on not allowed formatter * refactor weird try, except, finally pattern * change output handling * remove weird pattern * minor changes * move exception handling * use formatters as dictonary * minor changes * do not allow empty remotee * add output argument only if there are formatters * move exit codes * change default formatter * add init * conditional generators test with configure() (#7359) * conditional generators test with configure() * fixing test * fixing test * adding generators in the command line * #7328 Add 'outdated' column in search table (#7364) * #7328 Add 'outdated' column in search table Signed-off-by: Uilian Ries <uilianries@gmail.com> * #7328 do not remove more than requested Signed-off-by: Uilian Ries <uilianries@gmail.com> * #7328 Separate functional test Signed-off-by: Uilian Ries <uilianries@gmail.com> * #7328 Update other test to support outdated column Signed-off-by: Uilian Ries <uilianries@gmail.com> * #7364 No need for an additional test (#7373) * implementing __contains__ for option in self.info.options (#7303) * implementing __contains__ for option in self.info.options * add test * more tests * clarified test * changed self.info.options.whatever to raise * improve tests * fix tests * Update conans/test/functional/package_id/package_id_test.py Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> * Update conans/test/functional/package_id/package_id_test.py Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> * fixed test Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> * minor pep8 in conan_api.py * contribute test Co-authored-by: Javier G. Sogo <jgsogo@gmail.com> Co-authored-by: James <james@conan.io> Co-authored-by: Carlos Zoido <mrgalleta@gmail.com> Co-authored-by: Uilian Ries <uilianries@gmail.com>

implementing __contains__ for option in self.info.options

ac23325

memsharded added this to the 1.28 milestone Jul 2, 2020

memsharded mentioned this pull request Jul 2, 2020

[bug] Infinite recursion when checking for an option in the package_id method #7299

Closed

memsharded added 2 commits July 3, 2020 01:16

add test

5c93735

more tests

34fb213

jgsogo suggested changes Jul 3, 2020

View reviewed changes

memsharded added 5 commits July 14, 2020 12:24

Merge branch 'develop' into feature/option_in_info

a460504

clarified test

6ae2914

changed self.info.options.whatever to raise

a357ae7

improve tests

2d130f5

fix tests

aafe4de

memsharded requested a review from jgsogo July 14, 2020 12:34

memsharded commented Jul 14, 2020

View reviewed changes

memsharded assigned jgsogo Jul 15, 2020

jgsogo suggested changes Jul 16, 2020

View reviewed changes

memsharded and others added 3 commits July 16, 2020 17:34

Update conans/test/functional/package_id/package_id_test.py

b0de30f

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

Update conans/test/functional/package_id/package_id_test.py

85a1a97

Co-authored-by: Javier G. Sogo <jgsogo@gmail.com>

fixed test

9b4b513

jgsogo approved these changes Jul 16, 2020

View reviewed changes

jgsogo requested a review from danimtb July 16, 2020 17:16

jgsogo assigned danimtb and unassigned jgsogo Jul 16, 2020

danimtb approved these changes Jul 17, 2020

View reviewed changes

memsharded merged commit f563439 into conan-io:develop Jul 17, 2020
2 checks passed

memsharded deleted the feature/option_in_info branch July 17, 2020 12:08

memsharded mentioned this pull request Aug 28, 2020

Conan 1.28 breaking change (model/options.py/class PackageOptionValues) : Add config variable (conan.conf/environment variable) to select old/new behavior ? #7609

Open

ericLemanissier mentioned this pull request Nov 26, 2020

fix dependency options reading conan-community/conan-opencv#66

Open

implementing contains for option in self.info.options #7303

implementing contains for option in self.info.options #7303

memsharded commented Jul 2, 2020 •

edited by czoido

jgsogo left a comment

memsharded commented Jul 3, 2020

jgsogo commented Jul 3, 2020

memsharded Jul 14, 2020

memsharded Jul 14, 2020

jgsogo Jul 16, 2020

memsharded Jul 17, 2020

memsharded Jul 14, 2020

jgsogo Jul 16, 2020

jgsogo left a comment

jgsogo Jul 16, 2020

jgsogo Jul 16, 2020

		if self.full_options.shared:
		if "shared" in self.full_options and self.full_options.shared:

implementing __contains__ for option in self.info.options #7303

implementing __contains__ for option in self.info.options #7303

Conversation

memsharded commented Jul 2, 2020 • edited by czoido

jgsogo left a comment

Choose a reason for hiding this comment

memsharded commented Jul 3, 2020

jgsogo commented Jul 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgsogo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

implementing contains for option in self.info.options #7303

implementing contains for option in self.info.options #7303

memsharded commented Jul 2, 2020 •

edited by czoido