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.

Sign up for GitHub

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

Jump to bottom

Updating imagl recipe for conan 2.0 #16698

Closed
wants to merge 0 commits into from
Closed

Updating imagl recipe for conan 2.0 #16698

wants to merge 0 commits into from

Conversation

Woazim
Copy link
Contributor

@Woazim Woazim commented Mar 25, 2023

imagl/0.2.1

I've updated this recipe for conan 2.0. Most of changes are induced from cmake_lib template.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
@prince-chrismc prince-chrismc mentioned this pull request Mar 28, 2023
Open
jwillikers
jwillikers suggested changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@jwillikers jwillikers left a comment

Choose a reason for hiding this comment

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

There's a few things that should be cleaned up, but it looks pretty good already. Thanks!

recipes/imagl/all/conanfile.py Outdated

required_conan_version = ">=1.33.0"
required_conan_version = ">=1.59"


class ImaglConan(ConanFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

The homepage attribute should be https://gitlab-lepuy.iut.uca.fr/opengl/imagl, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both are OK. https://gitlab-lepuy.iut.uca.fr/opengl/imagl is mirrored to https://github.com/Woazim/imaGL.
I think that people are more used to github than to my own GitLab instance.

Copy link
Member

@uilianries uilianries Sep 6, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@Woazim in case https://github.com/Woazim/imaGL is a mirror, please, revert the conandata.yml. Mirror are welcome, but not as main link and they should keep same checksum: https://github.com/conan-io/conan-center-index/blob/master/docs/adding_packages/conandata_yml_format.md#mirrors

My mistake, I misread the statement, you are the real author actually.

recipes/imagl/all/conanfile.py Outdated

required_conan_version = ">=1.33.0"
required_conan_version = ">=1.59"


class ImaglConan(ConanFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

The License is also wrong. It should be lgpl-3.0-only from what I can tell.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right!!! What a mistake!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I messed up there by writing the lgpl part of the license in lowercase. It should probably be uppercase, i.e. LGPL-3.0-only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've just made the change. I hope that this is what blocks CI build with 'Invalid JSON String"...
By the way, I've changed source urls to github ones since my gitlab instance is temporarily down 😢

recipes/imagl/all/conanfile.py Outdated
@@ -24,18 +29,9 @@ class ImaglConan(ConanFile):
"with_png": True,
"with_jpeg": True,
}
generators = "cmake"
exports_sources = ["CMakeLists.txt", "patches/**"]
exports_sources = "patches/**"
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove this line? It shouldn't be necessary when using export_conandata_patches.

Suggested change
exports_sources = "patches/**"

recipes/imagl/all/test_package/CMakeLists.txt Outdated

add_executable(example example.cpp)
target_link_libraries(example ${CONAN_LIBS})
target_link_libraries(example imagl::imagl)
set_target_properties(example PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
set_target_properties(example PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON)
target_compile_features(example PRIVATE cxx_std_20)

recipes/imagl/all/test_v1_package/CMakeLists.txt Outdated
Comment on lines 1 to 9
cmake_minimum_required(VERSION 3.1)
project(PackageTest CXX)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()

add_executable(example ../test_package/example.cpp)
target_link_libraries(example ${CONAN_LIBS})
set_target_properties(example PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cmake_minimum_required(VERSION 3.1)
project(PackageTest CXX)
include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup()
add_executable(example ../test_package/example.cpp)
target_link_libraries(example ${CONAN_LIBS})
set_target_properties(example PROPERTIES CXX_STANDARD 20 CXX_STANDARD_REQUIRED ON)
cmake_minimum_required(VERSION 3.15)
project(test_package)
include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)
add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/../test_package/
${CMAKE_CURRENT_BINARY_DIR}/test_package/)

recipes/imagl/all/conanfile.py Outdated

self._cmake.definitions["STATIC_LIB"] = not self.options.shared
self._cmake.definitions["SUPPORT_PNG"] = self.options.with_png
get(self, **self.conan_data["sources"][self.version], destination=self.source_folder, strip_root=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified.

Suggested change
get(self, **self.conan_data["sources"][self.version], destination=self.source_folder, strip_root=True)
get(self, **self.conan_data["sources"][self.version], strip_root=True)

recipes/imagl/all/conanfile.py Outdated

required_conan_version = ">=1.33.0"
required_conan_version = ">=1.59"
Copy link
Contributor

Choose a reason for hiding this comment

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

It doesn't look like 1.59 is necessary here.

Suggested change
required_conan_version = ">=1.59"
required_conan_version = ">=1.53.0"

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot

This comment has been minimized.

Sign in to view
jwillikers
jwillikers previously approved these changes Apr 11, 2023
View reviewed changes
@Woazim
Copy link
Contributor Author

Woazim commented Apr 11, 2023

It seems that Windows builds are not starting... Why?

@conan-center-bot

This comment has been minimized.

Sign in to view
@CLAassistant
Copy link

CLAassistant commented May 18, 2023 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@conan-center-bot

This comment has been minimized.

Sign in to view
@Woazim Woazim mentioned this pull request Jun 6, 2023
Closed
@conan-center-bot conan-center-bot requested a review from danimtb June 20, 2023 12:00
@danimtb danimtb self-assigned this Jun 26, 2023
@conan-center-bot

This comment has been minimized.

Sign in to view
@prince-chrismc prince-chrismc mentioned this pull request Jun 26, 2023
Merged
@AbrilRBS AbrilRBS removed the request for review from franramirez688 June 29, 2023 12:10
@conan-center-bot

This comment has been minimized.

Sign in to view
This was referenced Jul 22, 2023
Closed
Closed
@ghost
Copy link

ghost commented Jul 22, 2023 •
edited by ghost
Loading

I detected other pull requests that are modifying imagl/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

jwillikers
jwillikers suggested changes Jul 22, 2023
View reviewed changes
recipes/imagl/all/conanfile.py Outdated

required_conan_version = ">=1.33.0"
required_conan_version = ">=1.59"


class ImaglConan(ConanFile):
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I messed up there by writing the lgpl part of the license in lowercase. It should probably be uppercase, i.e. LGPL-3.0-only.

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot
Copy link
Collaborator

Conan v1 pipeline ✔️

All green in build 4 (9a6e93908d2c64e4b7889bac7636d200f343fb3b):

  • imagl/0.1.0@:
    All packages built successfully! (All logs)

  • imagl/0.2.1@:
    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)

Conan v2 pipeline ❌

Note: Conan v2 builds may be required once they are on the v2 ready list

The v2 pipeline failed. Please, review the errors and note this will be required for pull requests to be merged in the near future.

See details:

Failure in build 5 (9a6e93908d2c64e4b7889bac7636d200f343fb3b):

  • imagl/0.1.1@:
    Invalid JSON String

  • imagl/0.1.0@:
    Invalid JSON String

  • imagl/0.1.2@:
    Invalid JSON String

  • imagl/0.2.1@:
    Invalid JSON String

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@uilianries uilianries mentioned this pull request Sep 6, 2023
Merged
3 tasks
@uilianries uilianries closed this Sep 6, 2023
@uilianries
Copy link
Member

Unfortunately Github closed this PR when I pushed some fixes to the original branch and I can not re-open it!

@danimtb
Copy link
Member

danimtb commented Sep 6, 2023

We have recollected the commits of this PR in a new PR at #19614

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uilianries uilianries uilianries left review comments

@jwillikers jwillikers jwillikers approved these changes

@danimtb danimtb Awaiting requested review from danimtb

@AbrilRBS AbrilRBS Awaiting requested review from AbrilRBS

@franramirez688 franramirez688 Awaiting requested review from franramirez688

Assignees

@danimtb danimtb

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Woazim @conan-center-bot @CLAassistant @uilianries @danimtb @jwillikers