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

compiler.version semantics and GCC/Clang versioning quirks #1214

Closed
himikof opened this issue Apr 17, 2017 · 13 comments
Closed

compiler.version semantics and GCC/Clang versioning quirks #1214

himikof opened this issue Apr 17, 2017 · 13 comments
Milestone

Comments

@himikof
Copy link

himikof commented Apr 17, 2017

Conan currently uses MAJOR.MINOR as the compiler.version ABI compatibility key for GCC/Clang, and this key is included in package_id by default. As far as I understand, this choice is a reasonable compromise: it allows seamlessly upgrading, for example, GCC 4.7.2 to GCC 4.7.3 (which is a bugfix-only release, with no new features), and distinguishes packages built with GCC 4.7 and 4.8 (a normal release with many new features, so the user could care about that). The same logic applies to Clang versioning scheme.
But recently, both GCC >= 5 and Clang >= 4 have changed the semantics of their version numbers. Now major version number will increase on any non-bugfix release. While Clang made their versioning change backwards-compatible (there will be no Clang 4.1.x or 5.1.x ever, just X.0.Y), GCC took a different approach: GCC 5.2 is a small bugfix release to 5.1 (and there is no 5.0 release, but that does not matter much).

So, using the current default compiler.version detection has entirely different and surprising behavior on GCC >= 5. You cannot reuse binary packages over a bugfix-only compiler version difference, and the cmake generator even checks that actual compiler version matches compiler.version, which prohibits using compiler.version=6.2 as the equality class for GCC 6.x. It could be patched individually for each package (including all dependencies) by manually specifying compiler.version=6 in package_id method, but this is a laborious workaround.

I would argue that the default behavior should be specified in terms of having more or less the same semantics on all compilers, not by just enforcing a formal rule with MAJOR.MINOR. In case of GCC, that would require making compiler.version equal X for GCC X.Y where X >= 5, and adjusting compiler version checks to handle more flexible version comparison. The same could be done for Clang >= 4, but luckily X.0 would still work just fine.

I understand that there could be some backwards compatibility concerns over changing the default compiler detection logic. At the very least, making the suggested behavior a global configuration opt-in would make adoption of Conan at my company much easier (GCC 5.x and 6.x are widely used here).

This is probably related to #1136 and #1127.

@memsharded
Copy link
Member

A small tip: If you are using one compiler version and want to install a certain different compiler version for one package, you could use package-settings like conan install -s compiler.version=6.3 -s MyPkg:compiler.version=6.2, no need to patch the recipes.

I think this issue has little to do with compiler detection. The compiler detection that is run when conan is first executed, it is just a guess to initialize <userhome>/.conan/conan.conf defaults. But conan does not detect or assume anything about the compiler or compiler version in any command. You can install packages for one compiler version without having that compiler installed. This issue it is just about ABI compatibility, if I understood it properly.

Also, for adoption in your company, not sure if you know that the settings can be fully customized. You can edit the file <userhome>/.conan/settings.yml, to define any ABI compatibility for your needs. That assumes that you will have your own server with your private packages, and not rely that much on conan.io packages (which is the most used approach by companies).

You are right, changing the default settings.yml could be problematic for backwards compatibility with existing conan.io packages. In any case, the biggest problem with the current settings defaults is that it could generate more binaries than necessary. Though this could be questionable, because even if theoretically ABI compatible over the MINOR, maybe the bug was already compiled into the binary, or some performance improvements were done, so it is interesting to have both 6.2 and 6.3, for example, even if the 6.2 would be ABI compatible with 6.3.

@himikof
Copy link
Author

himikof commented Apr 18, 2017

Thank you for the detailed response.
I did not think about overriding compiler.version per-package, it is a useful workaround for some cases.
I also missed that settings.yml restricts possible values for settings.

I would like to set compiler.version in <userhome>/.conan/conan.conf to major GCC version (manually, if changing autodetection is problematic).
But there are a couple things preventing this from working, as I understand:

  • Check in cmake generator for compiler.version to match output of gcc -dumpversion (truncated to MAJOR.MINOR). So it complains: Incorrect 'gcc' version 'compiler.version=6' is not the one detected by CMake: 'GNU=6.3'
  • Default settings.yml does not contain allowed values 5 and 6 for GCC versions. It could be added manually, of course, but I would suggest adding them by default, to show an "official" way to build packages with the same level of compiler version sensitivity with recent GCC.

I understand that there could be use for having both 6.2 and 6.3 packages, but it's just the same as having both 4.7.2 and 4.7.3, even in the number of packages over time. It is just a matter of reasonable compromise.

@memsharded
Copy link
Member

I think that in order to make it work, you should:

  • Change your settings.yml file, to match the patterns you want, like 6 instead of 6.2 & 6.3
  • Change your conan.conf to the default compiler.version=6 (or always use -s compiler.version=6)
  • Don't call conan_basic_setup() which includes the compiler check, from your CMakeLists, but skip it. You can copy its contents, create your own macro that removes the call to conan_check_compiler() (not sure if CMake can do it, but maybe you can override it before calling conan_basic_setup())

The issue is that we cannot change the current settings.yml, as it would break for a ton of users. Also I feel that the current scheme is slightly better than the previous one, i.e. having packages for 6.2 and 6.3 by default could be a good thing, more on the safe side than the opposite. As always, it is a trade-off, having more binaries, but having binaries built with the latest minor compiler version and not stick always just to the packages built with an older version.

@memsharded
Copy link
Member

We are facing this while upgrading some packages and updating conan-package-tools.

Indeed it is a bit of inconvenience, because minors are being frequently released, and the bad thing is that distros are just replacing the old one, like gcc 7.1 with the new gcc 7.2, so it is no longer easy to install 7.1 at all (in fact, it seems rather complicated).

We are thinking to do the next changes to conan:

  • Add 5, 6, 7 as valid compiler versions to settings.yml
  • Leave existing minors 7.1, 6.3, etc., versions, to not badly break things
  • Fix auto-detection to generate these values for default profile
  • Fix possible warnings and errors in the cmake version checks.

Doing this we would achieve what was already achieved with older gcc 4.9.X versions, in which the compiler could be updated without issues, and packages were compatible.

@lasote
Copy link
Contributor

lasote commented Oct 30, 2017

For https://github.com/conan-io/conan-docker-tools we would generate tags for conangcc5, conangcc6, conangcc7 and just use the available, abandoning the minor version (we could keep the existing images, but not generating new ones)

@sztomi
Copy link
Contributor

sztomi commented Oct 30, 2017

In our case, we generate a settings.yml with our init script and we don't depend on OS-installed toolchains. For us, it's best if the checks are strict because we want to avoid situations where a toolchain outside our packages is used accidentally.

For OSS usage, it's probably best to make it less strict.

@memsharded
Copy link
Member

Yes, I think proposed model will support both cases. If you want to create packages for 7.1, you can, and it should be consistent, and if you do for 7, then you will get the less strict behavior.

@lasote
Copy link
Contributor

lasote commented Oct 30, 2017

Thanks @sztomi. I think the check could take into account if you have declared in compiler.version only the major version, and not raise if the major matches (and gcc >= 5). So we could keep the same check if you declare your compiler version with a minor: compiler.version=7.1.
You could keep using your autogenerated settings.yml without any behavior change.

@acgetchell
Copy link

I just started looking into this, but if I'm understanding correctly that the default behavior for compiler.version=7 is to allow {gcc7.1, gcc7.2, ...} I'm all for it. I prefer not setting up profiles if I can possibly help it, that's one of the annoyances of working with conda.

@lasote
Copy link
Contributor

lasote commented Oct 31, 2017

Hi, Adam nice to read you! To summarize, the purposed behavior is:

  • In a fresh installation, conan autogenerate your default profile detecting your gcc >=7 as compiler.version=7(same for 5, 6 and 7, same for clang >=4) (For conan devs: Requires a little new feature in the detect.py module)
  • In a fresh installation, the default settings.yml will contain 5,6,7 as valid versions of gcc, keeping the existing ones too. (For conan devs: Requires changing the default template for settings.yml)
  • If you install a package selecting compiler.version=7 conan will generate a different binary package, won't reuse any package already created for compiler.version=7.1. It doesn't require any new feature in Conan, is the normal behavior.
  • CMake generator won't complain if the specified setting is compiler.version=7 and the detected compiler is 7.1. (For conan devs: It requires changing a little the compiler check in conan.)
  • CMake generator won't complain either if the specified setting is compiler.version=7.1 and the detected compiler is 7.1, but will complain if the minor doesn't match.
  • If your gcc is upgraded to 7.3, you can keep using your compiler.version=7 packages without any difference, because the specified setting is the same, 7.

@lasote lasote modified the milestones: 0.29, 0.30 Nov 21, 2017
@memsharded
Copy link
Member

Anyone knows the semantics of apple-clang?
Conan is using the apple-clang version, basically Apple LLVM version in https://gist.github.com/yamaya/2924292.

Is 8.1 just a bug-fix release wrt 8.0?

Many thanks!

@memsharded
Copy link
Member

This should be fixed now:

  • Added gcc 5, 6, 7 as accepted values in settings
  • Using gcc 5, 6, 7 as default values after detect
  • Making conanbuildinfo.cmake do not reject them as invalid for 5.X, 6.X, etc
  • Clang seems mostly ok, because conan already has 4.0, 5.0, and they are always doing 4.0.X (minor will always be 0)
  • Keeping apple-clang as is, without finding more information about compatibility, better be safe.

@memsharded
Copy link
Member

Released in 0.30, from now, conan should use the majors for gcc and clang by default, not the minors.

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

No branches or pull requests

6 participants