Add imagl/0.2.1 #4630

Woazim · 2021-02-20T23:42:43Z

Specify library name and version: imagl/0.2.0

I've read the guidelines for contributing.
I've followed the PEP8 style guides for Python code in the recipes.
I've used the latest Conan client version.
I've tried at least one configuration locally with the
conan-center hook activated.

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Prepare for CCI

recipes/imagl/all/conanfile.py

Woazim · 2021-02-21T23:10:40Z

Thanks @prince-chrismc. I will make some changes to take your advices into account. But there are compiling issues since last month, jenkins bot used Visual Studio 16.8 (according to logs). Now, it uses Visual Studio 16.4... I was surprised to find that my code was not compatible with this version, since Microsoft has moved a feature between two header files.

So I need to be more specific in the recipe to check the MSVC compiler version.

prince-chrismc · 2021-02-22T01:17:17Z

I was surprised to find that my code was not compatible with this version, since Microsoft has moved a feature between two header files.

🙃

We have this

conan-center-index/recipes/tl/all/conanfile.py

Line 32 in 95d0575

def lazy_lt_semver(v1, v2):

pattern if you need above a miniumum minor version

…rty, hardly detecting msvc compiler version

recipes/imagl/all/conanfile.py

…not work on my computer...

recipes/imagl/all/conandata.yml

recipes/imagl/all/conanfile.py

recipes/imagl/all/test_package/conanfile.py

recipes/imagl/all/conanfile.py

…nts on CMake's Ninja generator need

prince-chrismc

Thanks for adding comments, I just want to clarity one thing you wrote

prince-chrismc · 2021-04-29T21:27:24Z

recipes/imagl/all/conanfile.py

@@ -117,10 +133,31 @@ def requirements(self):
    def _configure_cmake(self):
        if self._cmake:
            return self._cmake
+
+        # CMake generator must be set to Ninja when using Visual Studio to let choose the right 
+        # MSVC compiler version when multiple versions are installed on build machine. The problem


The lingo I am most familiar with is "MSVC toolsets" and yes CMake which takes "VS2019" as an arg always chooses the highest one installed.

In my opinion, "MSVC toolset" is more often understood as the main version of toolset (140 for VS2013, 141 for VS2017 and 142 for VS2019). This is why I wanted to be explicit about the version of the compiler (the "142" toolset includes the 14.2x versions of the toolset, each version of which contains the MSVC compiler in version 19.2x ...).

prince-chrismc · 2021-04-29T21:29:26Z

recipes/imagl/all/conanfile.py

+        # Building with specific MSVC version could be needed since imaGL requires some minimal 
+        # version to be compiled according to its own version. Following table links imaGL version 


Is this required for the build to pass on CCI?

In the Conan universe if you want to build an exact toolset you could use the new msvc compiler. (this is consistent with CMake)

Just as a sanity check,

Is this required ? Or is it an improvement?

No, it's not required to pass on CCI. By the way, CCI is not able to build imaGL since tools.vswhere does not work on CCI (see #4954) and I don't know how to test for minor version of MSVC without vswhere. It's done from line 89 to 95. Some imaGL versions could be built in CCI (CCI has Visual Studio 16.4, so imaGL 0.1.2 and 0.2.1 could be built), but I don't know how to be sure that the recipe is cooked on the CCI CI/CD machine.

I do this in the recipe to allow imaGL to be built locally in a Visual Studio command line environment with a specific version of the toolset (using VCVARS and company environment variables). So the recipe is compatible with "Visual Studio" compiler from the Conan universe.

Still in this universe, using the new msvc compiler is allowed thanks to lines 47 and 53. But CCI still uses "Visual Studio".

jgsogo

I think we need to understand a couple of things that are being implemented in this recipe. It deserves some attention if we want it to be merged.

I have questions related to:

option allow_clang_11: I think it is not needed.
MSVC minor versions: if needed, we can upgrade the version, now we are using 19.28.29333. But the recipe cannot use vswhere in the validate() method because that method is running first in a Linux container... it could be used in the build() if it is really needed.

jgsogo · 2021-05-07T12:48:40Z

recipes/imagl/all/conanfile.py

+                "clang": "10",
+                "apple-clang": "11"
+        }
+        if tools.Version(self.version) <= "0.1.1" or tools.Version(self.version) == "0.2.0":


Version 0.2.0 is not listed in config.yml... is this a typo?

jgsogo · 2021-05-07T12:48:56Z

recipes/imagl/all/conanfile.py

        }
+        if tools.Version(self.version) <= "0.1.1" or tools.Version(self.version) == "0.2.0":


Version 0.2.0 is not listed in config.yml... is this a typo?

jgsogo · 2021-05-07T12:49:26Z

recipes/imagl/all/conanfile.py

+
+    @property
+    def _supports_jpeg(self):
+        return tools.Version(self.version) >= "0.2.0"


Version 0.2.0 is not listed in config.yml... is this a typo?

it's fine for me, I guess that jpeg support was added in 0.2.0, so it doesn't matter that exact 0.2.0 version was not in CCI, it makes the recipe more robust if for any reason 0.2.0 is added later.

We started by adding 0.2.0 but there was a new point release since then

jgsogo · 2021-05-07T12:57:30Z

recipes/imagl/all/conanfile.py

+                    version="[{}.0,{}.0)".format(str(self.settings.compiler.version), int(str(self.settings.compiler.version))+1), latest=True, property_="installationVersion")[0]["installationVersion"]
+                toolset_version = tools.vcvars_dict(self).get("VCToolsVersion", os.environ.get("VCToolsVersion",""))
+            except ConanException:
+                raise ConanInvalidConfiguration("Your Visual Studio compiler seems not to be installed in a common way. It is not supported. This unfortunately happens in conan center index. To build imaGL, append '--build missing' to your 'conan install' command line.")


What is the purpose here? Probably we need a different approach. These lines are evaluated in the machine where Conan is running, in CCI we will run them in a Linux container which will not work as expected, and probably discard the intended configuration.

In the validate() we can use only the settings/options from the profile and dependencies. This method should be agnostic to everything else.

jgsogo · 2021-05-07T13:06:07Z

recipes/imagl/all/conanfile.py

+        if self.options.shared:
+            del self.options.fPIC
+        if not self.settings.compiler.cppstd:
+            self.settings.compiler.cppstd = "20"


Do not assign a value here, it is not going to work... and future versions of Conan will fail here.

Woazim · 2021-05-11T17:07:28Z

To answer to @jgsogo

option allow_clang_11: I think it is not needed.

This has already been discussed here. For short, it's because of a misinstallation in CCI for clang 11. See conan-io/conan-docker-tools#251 and conan-io/conan-docker-tools#252.

MSVC minor versions: if needed, we can upgrade the version, now we are using 19.28.29333. But the recipe cannot use vswhere in the validate() method because that method is running first in a Linux container... it could be used in the build() if it is really needed.

Ok, I understand now why it does not 'work'. So, since you are saying that you are using 19.28, I will totally remove this check which does not make sense anymore. Minimal version to build imaGL is 19.25 and CCI documentation prints that MSVC is 19.24 (Visual Studio 16.4)

…ppstd

…es MSVC 19.28

conan-center-bot · 2021-05-11T22:07:22Z

All green in build 19 (0aab73292abef94d877b22b06baf31281322fc50):

imagl/0.1.0@:
All packages built successfully! (All logs)
imagl/0.1.1@:
All packages built successfully! (All logs)
imagl/0.1.2@:
All packages built successfully! (All logs)
imagl/0.2.1@:
All packages built successfully! (All logs)

Woazim and others added 12 commits January 12, 2021 14:07

Adding imagl recipe

e9078dd

Adding configuration requirements in configure()

8376ef5

Prepare for CCI

34a13cf

Signed-off-by: Uilian Ries <uilianries@gmail.com>

Merge pull request #1 from uilianries/imagl/0.1.0

e12e215

Prepare for CCI

Update conanfile.py

109a45e

Merge remote-tracking branch 'upstream/master'

a391575

Setting minimum Visual Studio to 16 to let CCI work.

906a478

removing fPIC option for Windows config

a5c8d70

Merge branch 'master' of https://github.com/conan-io/conan-center-index

37ea45e

Adding imaGL 0.1.1

4c90e58

Merge branch 'master' of https://github.com/conan-io/conan-center-index

1345d65

Adding imagl 0.2.0

e2ad08a

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label Feb 21, 2021

This was referenced Feb 21, 2021

Updatable recipes qchateau/conan-center-bot#1

Open

[request] Updatable recipes #3470

Closed

prince-chrismc suggested changes Feb 21, 2021

View reviewed changes

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

Deleting unsupported option in configure, adding _supports_jpeg prope…

5028584

…rty, hardly detecting msvc compiler version

This comment has been minimized.

Sign in to view

Woazim commented Feb 26, 2021

View reviewed changes

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

Nicer way to get MSVC version.

065ea9b

This comment has been minimized.

Sign in to view

maybe this one, which directly comes from other recipes. But it does …

4932cd9

…not work on my computer...

This comment has been minimized.

Sign in to view

prince-chrismc reviewed Feb 26, 2021

View reviewed changes

recipes/imagl/all/conandata.yml Outdated Show resolved Hide resolved

prince-chrismc reviewed Feb 26, 2021

View reviewed changes

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

More checks on clang and Visual, according to last building failures.

12c9114

Woazim changed the title ~~Add imagl/0.2.0~~ Add imagl/0.2.1 Feb 26, 2021

conan-center-bot removed the Failed label Mar 29, 2021

prince-chrismc previously approved these changes Mar 29, 2021

View reviewed changes

github-actions bot mentioned this pull request Mar 29, 2021

Pull Requests Ready to be Reviewed prince-chrismc/conan-center-index-pending-review#1

Open

madebr reviewed Apr 7, 2021

View reviewed changes

recipes/imagl/all/test_package/conanfile.py Outdated Show resolved Hide resolved

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

recipes/imagl/all/conanfile.py Outdated Show resolved Hide resolved

prince-chrismc referenced this pull request in prince-chrismc/conan-center-index-pending-review Apr 14, 2021

update cpr to use raw-data branch

bb4d836

prince-chrismc referenced this pull request in prince-chrismc/conan-center-index-pending-review Apr 15, 2021

Update pending_review.go

aefe3c4

Code formatting correction, warning when using allow_clang_11 & comme…

0dcdf20

…nts on CMake's Ninja generator need

Woazim dismissed prince-chrismc’s stale review via 0dcdf20 April 28, 2021 22:19

This comment has been minimized.

Sign in to view

prince-chrismc reviewed Apr 29, 2021

View reviewed changes

conan-center-bot requested review from uilianries, memsharded and jgsogo May 4, 2021 12:00

jgsogo suggested changes May 7, 2021

View reviewed changes

conan-center-bot requested a review from danimtb May 11, 2021 11:57

Removing vswhere in validate() and affectation to settings.compiler.c…

3476bcb

…ppstd

This comment has been minimized.

Sign in to view

conan-center-bot added the Failed label May 11, 2021

Removing Ninja generator forcing which is no more needed since CCI us…

0aab732

…es MSVC 19.28

conan-center-bot removed the Failed label May 11, 2021

conan-center-bot requested a review from SSE4 May 13, 2021 11:57

danimtb approved these changes May 13, 2021

View reviewed changes

jgsogo approved these changes May 18, 2021

View reviewed changes

conan-center-bot requested a review from lasote May 18, 2021 11:59

prince-chrismc approved these changes May 18, 2021

View reviewed changes

conan-center-bot merged commit 496fd2b into conan-io:master May 18, 2021

SSE4 approved these changes May 18, 2021

View reviewed changes

OleksandrKvl mentioned this pull request May 10, 2023

recipe for sbepp/1.0.1 #16938

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add imagl/0.2.1 #4630

Add imagl/0.2.1 #4630

Woazim commented Feb 20, 2021

This comment has been minimized.

Woazim commented Feb 21, 2021

prince-chrismc commented Feb 22, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a comment

prince-chrismc Apr 29, 2021

Woazim Apr 30, 2021

prince-chrismc Apr 29, 2021

Woazim Apr 30, 2021

jgsogo left a comment

jgsogo May 7, 2021

jgsogo May 7, 2021

jgsogo May 7, 2021

SpaceIm May 7, 2021

prince-chrismc May 7, 2021

jgsogo May 7, 2021

jgsogo May 7, 2021

Woazim commented May 11, 2021

This comment has been minimized.

conan-center-bot commented May 11, 2021

		# Building with specific MSVC version could be needed since imaGL requires some minimal
		# version to be compiled according to its own version. Following table links imaGL version

		}
		if tools.Version(self.version) <= "0.1.1" or tools.Version(self.version) == "0.2.0":

Add imagl/0.2.1 #4630

Add imagl/0.2.1 #4630

Conversation

Woazim commented Feb 20, 2021

This comment has been minimized.

Woazim commented Feb 21, 2021

prince-chrismc commented Feb 22, 2021

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

prince-chrismc left a 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

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

Woazim commented May 11, 2021

This comment has been minimized.

conan-center-bot commented May 11, 2021