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

Intel compiler POC using compatible ids feature #6052

Merged
merged 35 commits into from Nov 22, 2019

Conversation

lasote
Copy link
Contributor

@lasote lasote commented Nov 11, 2019

Changelog: Feature: Support for Intel compiler.
Docs: conan-io/docs#1479

This branch is the original @danimtb one but updated and conflict resolved

  • Refactor of compatible packages, (can be extracted to a different PR if needed) I've completely broken the previous experimental approach.
  • Added helpers to the info so we can declare that a Visual package can be consumed with Intel and the opposite.
  • The settings model looks good as dani suggested. The approach code provided by @ohanar used this model and looks reasonable and could be added later to complete the Intel functionality in Conan.

#revisions: 1

Closes #5590
Related #5699
Supersedes #5626 and #5770 based on the compatible IDs feature.

@lasote lasote changed the title Feature/5590 compatible ids Intel compiler POC using compatible ids feature Nov 11, 2019
@lasote lasote self-assigned this Nov 11, 2019
Copy link
Member

@memsharded memsharded left a comment

I like the approach, but if I understand it correctly, it seems that you are removing the validation, is that correct?

@lasote
Copy link
Contributor Author

@lasote lasote commented Nov 13, 2019

Totally correct. My point is, it has to be a ConanInfo, otherwise, doing things like the test additional_id_mode_test won't be possible.
So, if we have the same interface and previously we allowed (feature, not bug) to do self.info.settings.compiler="gcc_between_4_and_8" we should allow exactly the same in the additional packages.
Also, to discuss again the interface of self.compatible_packages.append. Now that there is no special class, should we reconsider a cp = self.register_new_info() or similar?

@lasote lasote marked this pull request as ready for review Nov 13, 2019
@lasote lasote assigned memsharded and unassigned lasote Nov 13, 2019
Copy link
Member

@danimtb danimtb left a comment

I like the approach as it is more complete with the possibility of modeling compatible packages for the revision mode. However, I feel users will find it a little bit more difficult to understand at first sight.

Now I think the challenge is to write the docs especially for the last test in this PR

conans/client/conf/__init__.py Show resolved Hide resolved
conans/model/info.py Outdated Show resolved Hide resolved
@lasote lasote merged commit 7554cbd into conan-io:develop Nov 22, 2019
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants