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

Fix pkg-config description parser #8143

Merged
merged 5 commits into from Dec 2, 2020

Conversation

uilianries
Copy link
Member

@uilianries uilianries commented Dec 2, 2020

Changelog: Fix: Throw error when the recipe description is not a string.
Docs: Omit

fixes #8142

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
conans/test/functional/generators/pkg_config_test.py Outdated Show resolved Hide resolved
class PkgConfigConan(ConanFile):
name = "foo"
version = "0.1"
description = ("Conan, ",
Copy link
Member

Choose a reason for hiding this comment

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

Is a tuple an allowed description? I don't recall that it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Not a description, but a python feature. Months ago I watched a python meetup where that usage was more pythonic. And here is the reference: https://www.python.org/dev/peps/pep-0008/#maximum-line-length

The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses.

Copy link
Member

@memsharded memsharded Dec 2, 2020

Choose a reason for hiding this comment

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

That is without the comma @uilianries

description = ("Conan, "
               " the C++ package manager")

That will resolve to a single string, not a tuple

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, so it's more a prone error than a bug. Anyway, if we pass a tuple, we have an exception which doesn't help. So we could accept tuple, or raise a better error.

Copy link
Member

Choose a reason for hiding this comment

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

Lets make a better error. I don't see here a use case in which we want to support tuples, lists, or other different ways of inputting the data, it should always be a string.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@uilianries uilianries changed the title Reproduce pkg-config format error Fix pkg-config description parser Dec 2, 2020
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
Signed-off-by: Uilian Ries <uilianries@gmail.com>
@Croydon
Copy link
Contributor

Croydon commented Feb 28, 2021

This change breaks some existing recipes, like conan-io/conan-center-index#4726

Shouldn't such changes only happen with Conan 2.0?

@memsharded
Copy link
Member

Hi @Croydon

It is not a breaking change, it is a bug fix. As described in the documentation (https://docs.conan.io/en/latest/reference/conanfile/attributes.html#description), the description field is a string. Passing any other type, like a tuple, is an error and the recipe should fail (or could have undefined behavior if the error is not managed)

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.

[bug] Recipe description breaks pkg-config generator
3 participants