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

Add warning to deprecate path functions in conanfile #16247

Merged
merged 3 commits into from
May 14, 2024

Conversation

juansblanco
Copy link
Contributor

@juansblanco juansblanco commented May 14, 2024

Changelog: Feature: Deprecate use of path accessors in ConanFile.
Docs: omit

There's a duplicity between the path and folder functions in the conanfile, for example package_path returns the value of package_folder but using the Path library.
Also the path functions weren't documented, so they were not intended for use.

For this reasons we are deprecating all the path functions and encourage use of the folder functions instead. Warnings were added to notify users when using the path functions to use the folder functions instead.

Closes: #12173
Related to: conan-io/docs#2753

@juansblanco juansblanco added this to the 2.4.0 milestone May 14, 2024
conans/model/conan_file.py Outdated Show resolved Hide resolved
conans/model/conan_file.py Outdated Show resolved Hide resolved
conans/model/conan_file.py Outdated Show resolved Hide resolved
@czoido czoido merged commit 5ac19ca into conan-io:develop2 May 14, 2024
1 check passed
@shoeffner
Copy link
Contributor

shoeffner commented Jun 25, 2024

edit: Now I found the explanations here: #12173 (comment) , which clear some things up.

I am confused about this deprecation, as the related conan-io/docs#2753 only mentions arguments for using pathlib instead of against it, followed by a decision against it. I know that for some of the ..._folder variables, no ..._path exists, but I would have preferred to add those and thus extend the support for ..._paths in favor of going back to strings. I went through all places and replaced the use of Path(..._folder) with ..._path when I found out about it a few months ago as we were using Path(..._folder) almost exclusively anyways, but also since it had the extra benefit of failing with a descriptive message if a ..._folder was not set with the assert is not None -- now we get a TypeError, so I guess that's still okay but slightly less descriptive.

Luckily, resolving this deprecation is replacing all ..._path calls with Path(..._folder) again and adding a couple of imports, so this is not a big issue for us :)

@memsharded
Copy link
Member

I know that for some of the ..._folder variables, no ..._path exists, but I would have preferred to add those and thus extend the support for ..._paths in favor of going back to strings. I went through all places and replaced the use of Path(..._folder) with ..._path when I found out about it a few months ago as we were using Path(..._folder) almost exclusively anyways, but also since it had the extra benefit of failing with a descriptive message if a ..._folder was not set with the assert is not None -- now we get a TypeError, so I guess that's still okay but slightly less descriptive.

Yes, probably the biggest factor was the fact that passing Path objects to some Conan inputs (like self.folders, self.cpp_info) would be breaking behavior, they expect to receive old os.path strings, and the API was more problematic, as something like self.folders.generators = self.build_path / "generators" would be breaking things, then users would request this to be supported, but this would create some risks (some of them are attributes, not methods), etc.

Luckily, resolving this deprecation is replacing all ..._path calls with Path(..._folder) again and adding a couple of imports, so this is not a big issue for us :)

Sorry about that. Even if it sounded a good idea initially, it was a mistake, and we should have followed the Python zen, to try to have only 1 way of doing things, and as the old os.path strings cannot be really removed, the Path() alternatives add more clutter than value, specially if the input interface cannot follow easily. We are at a moment after the Conan 1->2 transition that things better be more stable, and things that are mostly stylistic like this one, should have never progressed to code (even if it was not documented) in the first place. Thanks for your understanding!

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.

[question] Is it safe to use self.package_path? ?
5 participants