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

Deprecate 'cppflags' in favor of 'cxxflags' in class CppInfo #4611

Merged
merged 1 commit into from Feb 27, 2019

Conversation

Projects
None yet
4 participants
@jgsogo
Copy link
Member

commented Feb 26, 2019

Changelog: Fix: Deprecate 'cppflags' in favor of 'cxxflags' in class CppInfo
Docs: conan-io/docs#1091

closes #4337

  • Update docs

In order to remove all the usages of the deprecated member in our codebase, I find one problem: the files written by the generators...

For example, the conanbuildinfo.txt file now contains

[cppflags_Hello]
FLAG

and the expected (for Conan v2.0) should be:

[cxxflags_Hello]
FLAG

During the transition, we can output something like the following one, to keep compatibility:

[cppflags_Hello]
FLAG
[cxxflags_Hello]
FLAG

So, our reader should be able to deal with the first option and the last one until Conan V2 (checking that both sections contain the same information). This is easy for the txt generator, but it might require some more testing for other generators.

Maybe we can merge this PR jsut with the deprecation notice as the issue states, and then we can open a different PR to fix all the usages of this deprecated code. WDYT?

@ghost ghost assigned jgsogo Feb 26, 2019

@ghost ghost added the stage: review label Feb 26, 2019

@jgsogo jgsogo force-pushed the jgsogo:issue/4337-cxxflags branch from b7d4585 to fad1d9d Feb 26, 2019

@jgsogo jgsogo marked this pull request as ready for review Feb 26, 2019

@jgsogo jgsogo requested a review from lasote Feb 26, 2019

@jgsogo jgsogo added this to the 1.13 milestone Feb 26, 2019

@memsharded
Copy link
Contributor

left a comment

Looks good, the only thing if we are going to remove it in Conan 2.0, we might want to raise an Exception, so defining it is not silently dismissed.

@danimtb

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

I think the approach proposed is right and the [cxxflags] could be introduced in another PR in the future as there are other generators with the same problem.

Please make sure to update the docs!

@jgsogo

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2019

@memsharded

Looks good, the only thing if we are going to remove it in Conan 2.0, we might want to raise an Exception, so defining it is not silently dismissed.

I would like to use the approach recommended by the deprecation package we are using, but I cannot get conans.__version__ in the build_info.py file (circular dependency). I can write a test on my own, but it is too much code:

    """
    TODO: Need to solve the circular dependency when importing `conans.__version__` in build_info.py
    @deprecation.fail_if_not_remove
    def test_deprecation_v2(self):
         cpp_info = CppInfo("roothpath")
         self.assertListEqual(cpp_info.cppflags, [])
    """

    def test_deprecation_v2(self):
        cpp_info = CppInfo("roothpath")

        import warnings
        from conans import __version__
        from packaging import version

        if version.parse(__version__) >= version.parse("2.0"):
            with warnings.catch_warnings(record=True) as caught_warnings:
                warnings.simplefilter("always")

                self.assertListEqual(cpp_info.cppflags, [])

                if caught_warnings:
                    self.fail("CppInfo::cppflags must be removed in Conan 2.0")

Other option is to move conans.__version__ from conans/__init__.py to conans/version.py to avoid the circular dependency, and use the commented version of the test

@lasote

lasote approved these changes Feb 26, 2019

Copy link
Contributor

left a comment

I'm not worried about raising an error in Conan 2.0, we will manage. Please add the docs to merge it.

@jgsogo

This comment has been minimized.

Copy link
Member Author

commented Feb 26, 2019

The idea is to implement a test that fails in Conan v2.0 (so it will remember us that we have to delete that deprecated functionality), but the code will keep working the same...

... on my way to docs!

@danimtb

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Docs are ready!

@danimtb danimtb merged commit 0a49789 into conan-io:develop Feb 27, 2019

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Feb 27, 2019

@jgsogo jgsogo deleted the jgsogo:issue/4337-cxxflags branch Feb 27, 2019

Croydon added a commit to bincrafters/bincrafters-conventions that referenced this pull request Apr 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.