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

[feature] Add cstd to compiler settings #13777

Open
1 task done
PhilippFinke1988 opened this issue Apr 26, 2023 · 7 comments · May be fixed by #16028
Open
1 task done

[feature] Add cstd to compiler settings #13777

PhilippFinke1988 opened this issue Apr 26, 2023 · 7 comments · May be fixed by #16028

Comments

@PhilippFinke1988
Copy link

PhilippFinke1988 commented Apr 26, 2023

Background
We encountered an issue with the GCC toolchain (at least GCC 9) pulling in BSD and SVr4 extensions if no C/C++ dialect via compiler flag -std= has been explicitly defined. This is unwanted on many embedded targets.

We found out that in contrast to the legacy Conan CMake build helper the new generate() automatically translates the infomation in setting compiler.cppstd to a toolchain specific flag and appends it to the compiler call. This works pretty well for C++ files, but as many of our packages contain C files or are written exclusively in C, there is no similar feature for C in Conan. Using the same flag as for C++ is no option as this will create warnings for C files.

Proposal

  • Add cstd to settings compiler similar to cppstd.
  • Translate the information in compiler.cstd to a compiler specific flag (e.g. for GCC compiler.cstd=11 translates to -std=c11).
  • If using the generate() approach add this compile flag to the compiler call.

Have you read the CONTRIBUTING guide?

  • I've read the CONTRIBUTING guide
@memsharded
Copy link
Member

Thanks for your proposal @PhilippFinke1988

I think this proposal makes sense, and we could consider it.
However there are some implications that would make this challenging to introduce:

  • compiler.cppstd has to be removed in conanfile.py recipes for packages that are pure C, because it doesn't apply and it is wrong.
  • If we introduced compiler.cstd, the opposite would happen, conanfile.py recipes for C++ packages would have to remove it, because otherwise it would be be affecting.
  • The number of C++ recipes is huge, this cannot be introduced without breaking.
  • There is a compatibility.py plugin default implementation dedicated to provide default binary compatibility for different compiler.cppstd. That should also be extended for compiler.cstd, but that would be a combinatoric explosion for most cases, exclusively C or C++ binaries (not a mixture of them, in that case, the combinatorics will still hit). We should investigate this and propose some solution.

That means that we would need some recipe opt-in for this feature:

  • We have been considering the languages = "C" (or "C++") recipe attribute for some time
  • This attribute can be used as an opt-in for further automation at the recipe level
  • The opt-in would be implemented by automatic removal of compiler.cstd if languages is not defined

Taking this complexity in mind, it will be challenging to prioritize this for the short-term, there would be quite a few other priorities. I am assigning this as 2.X roadmap, in the meantime, it might be approached by injection of tools.build:cflags, or might require hardcoding it in the build system scripts (basically what was largely done in Conan 1.X for cppstd)

@PhilippFinke1988
Copy link
Author

PhilippFinke1988 commented Apr 26, 2023

@memsharded Thanks for the fast reply and I'm happy to hear that you consider my proposal. You're totally correct that this is a rather complex feature request and I understand it cannot be done in the short-term. As we're currently still using Conan 1.X in our company and planning to migrate to 2.X in the long-term. So we'd be happy to see this feature in a future 2.X release. Until then we'll surely find a workaround for this issue based on the ideas you provided.

@PhilippFinke1988
Copy link
Author

@memsharded Is there already any decision to introduce the languages attribute? We're planning to migrate to Conan 2.X soon and would like to have this feature to avoid several migration iterations on all of our packages.

@memsharded
Copy link
Member

Thanks for the ping @PhilippFinke1988

We haven't been able to prioritize this yet, we have been very busy with the stabilization of 2.0, migration of ConanCenter to 2.0, and also releasing already 17 releases with some very demanded features (https://blog.conan.io) like package-lists, cache save/remote, sources backup, metadata files, etc.

I think we want to tackle this when possible, let me assign it a fixed milestone and a look-into label.

@memsharded memsharded modified the milestones: 2.X, 2.2 Jan 16, 2024
@memsharded
Copy link
Member

Resuming this to discuss with the team.

Alternatives:

  • settings.cstd vs settings.compiler.cstd. The former is easier to opt-in, 1 line declaration in settings.yml, but allows worse control of flags and validation from different compilers
  • Discuss the compiler.cstd defined in profile but discarded unless language = "C", ... to avoid breaking every C++ package
  • compatibility.py is still a bit too slow to do this, unless for language = "C" only, maybe? Should we prioritize the speed of this check?

@memsharded
Copy link
Member

Quick questions:

Would adding a new compiler.cstd subsetting that is automatically removed in every recipe unless language = "C", ... is defined in the recipe, could it be breaking somehow?

Would it be preferable that when a profile/cli defines compiler.cstd=xxx (not None), every package_id for every C++ package that do not depend on the cstd would change and require a new binary?

@memsharded memsharded modified the milestones: 2.2.0, 2.3.0 Mar 15, 2024
@memsharded memsharded linked a pull request Apr 5, 2024 that will close this issue
@memsharded
Copy link
Member

I am starting the proposal in #16028, testing and feedback welcome.

@memsharded memsharded modified the milestones: 2.3.0, 2.4.0 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants