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

Add swig/4.0.1 #1717

Merged
merged 7 commits into from
Jun 22, 2020
Merged

Add swig/4.0.1 #1717

merged 7 commits into from
Jun 22, 2020

Conversation

madebr
Copy link
Contributor

@madebr madebr commented May 23, 2020

Specify library name and version: swig/4.0.1

Ported from Bincrafters' recipe: https://github.com/bincrafters/conan-swig_installer

@rea-yseop Fixes bincrafters/community#1031 (please check)
@rea-yseop Fixes bincrafters/community#1174

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

@conan-center-bot
Copy link
Collaborator

Some configurations of 'swig/4.0.1' failed in build 1 (06840411a8e773cff41bf865c6df9b802919680c):

  • Windows x86_64, Debug, Visual Studio 14, MTd
    You are depending on 'winflexbison/2.5.18@bincrafters/stable' but it is not in the repository

@conan-center-bot
Copy link
Collaborator

Some configurations of 'swig/4.0.1' failed in build 2 (e34d0b160efdb159b18f82a716560194ef00a8dc):

@conan-center-bot
Copy link
Collaborator

All green in build 3 (409b5d97014b04c05bdecdfd909aa0996b1bc0ec)! 😊

This was referenced May 23, 2020
Closed
Closed
@madebr
Copy link
Contributor Author

madebr commented May 25, 2020 •
edited
Loading

@uilianries
What msvc runtime will be used?
See #1705

I don't want a debug swig build.
https://c3i.jfrog.io/c3i/misc/logs/pr/1717/3/swig/4.0.1/68351efafb45a2dbc5d988b1e4589a99163562ee/ shows that only Debug is built.

@uilianries
Copy link
Member

@madebr interesting, because the original version (Conan Community) doesn't offer that option. Perhaps msvc runtime is wrong there and we need to remove.

@madebr
Copy link
Contributor Author

madebr commented May 25, 2020 •
edited
Loading

@uilianries
That won't fix the problem. The package can still be built in debug mode, or with debug runtime.
I think all installer packages should throw a ConanInvalidConfiguration if "d" in compiler.runtime.

Conan community's version does not have this problem, because the build.py script explicitly only builds the Release version. The Release and Debug version have the same package_id, because compiler and build_type is removed in package_id.
See https://github.com/conan-community/conan-cmake-installer/blob/8964ec96ae8815e9f6961a07c5b2fb23a4281ef8/conanfile.py#L132-L137

@SSE4 SSE4 requested review from danimtb and uilianries May 26, 2020 17:02
@madebr
Copy link
Contributor Author

madebr commented May 27, 2020

I think adding build_type will avoid problems such as #1705.
Even for installer-only packages.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'swig/4.0.1' failed in build 4 (07e642b05479164a5c0d9492f831f634da9484ec):

@conan-center-bot
Copy link
Collaborator

All green in build 5 (2a0364bdff32923081857e6e01d8df4031b69e4c)! 😊

@conan-center-bot
Copy link
Collaborator

All green in build 6 (84ad51109b94675dc71ab11db2b8c7afcfa4de0b)! 😊

uilianries
uilianries reviewed Jun 1, 2020
View reviewed changes
recipes/swig/all/conanfile.py Outdated

def package_id(self):
# Verilator is a executable-only package, so the compiler version does not matter
del self.info.settings.compiler.version
Copy link
Member

Choose a reason for hiding this comment

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

very interesting ...

Copy link
Contributor Author

@madebr madebr Jun 1, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I think this recipe needs the same treatment as cmake: #1783

madebr
madebr commented Jun 17, 2020
View reviewed changes
recipes/swig/all/conanfile.py Outdated Show resolved Hide resolved
recipes/swig/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot
Copy link
Collaborator

All green in build 7 (1e43ca8380396ea0af73e6379153a5aa9d62b2da)! 😊

danimtb
danimtb reviewed Jun 17, 2020
View reviewed changes
recipes/swig/all/conanfile.py
return os.path.join(self.package_folder, "bin", "swiglib").replace("\\", "/")

def package_info(self):
# FIXME: Don't set cmake_find_package name because conan cmake generators do not define SWIG_EXECUTABLE
Copy link
Member

Choose a reason for hiding this comment

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

Is this <PKG-NAME>_EXECUTABLE something standard in CMake? What happens if the sources provide more than one executable? Asking this just to know if it makes sense to add this as a feature in the CMake generators

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I's quite common:

Also, some provide more than one _EXECUTABLE variable:

But there are exceptions:

Also, it would be interesting to provide these as targets:

@SSE4 SSE4 requested review from uilianries June 22, 2020 03:07
@uilianries uilianries merged commit a69c690 into conan-io:master Jun 22, 2020
@madebr madebr deleted the swig branch June 22, 2020 19:31
@danimtb danimtb mentioned this pull request Jun 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danimtb danimtb danimtb approved these changes

@uilianries uilianries uilianries approved these changes

@SSE4 SSE4 SSE4 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

swig_installer/4.0.1: Could NOT find SWIG (missing: SWIG_DIR) [swig_installer] unknown option with cmake
5 participants
@madebr @conan-center-bot @uilianries @SSE4 @danimtb