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

[bug] PR-15704 (print to stderr) breaks existing recipes #15932

Closed
sboehmann opened this issue Mar 22, 2024 · 4 comments · Fixed by #15934
Closed

[bug] PR-15704 (print to stderr) breaks existing recipes #15932

sboehmann opened this issue Mar 22, 2024 · 4 comments · Fixed by #15934
Assignees
Milestone

Comments

@sboehmann
Copy link

Describe the bug

In PR print to stderr the python's builtin print function was replaced and slightly refactored in Fix: remove existing 'file' arg when overwriting print.

I just updated to conan 2.2.1 which only contains the first PR. After this update I got error messages when building several recipes and spent a long time trying to understand and find the cause. The error message was
print() got multiple values for keyword argument 'file' and the stacktrace pointed to a simple print call in my recipe.

Up to this point there is an error message that is difficult to understand, but at least there is a concrete error.

The second PR, however, would have meant that I would not get any errors at all, but the result of my recipes would no longer be correct.

Let me explain. In many recipes some files have to be saved (extracted licenses, CMake, etc.). Sometimes the code looks like this:

with open(target_file) as f:
    print(content, file=f)

In conan 2.2.1 this code leads to the error message mentioned above. As I said, difficult to understand and not nice.

But in the upcoming conan version, this does not lead to any error message at all. Instead, the content that should actually be written to the file is output to stderr and the file remains empty.

How to reproduce it

No response

@memsharded memsharded self-assigned this Mar 22, 2024
@memsharded
Copy link
Member

Hi @sboehmann

Thanks for your report.

There was indeed some changes in latest Conan release, to avoid print() in recipes to mess with the stdout, because it was breaking all json outputs when recipes decide to add some print() statements.

This usage of print() was never documented or supported (to be fair, it is not explicitly forbidden in the docs), and the recommendation is to use save(self, target_file, content) as documented in https://docs.conan.io/2/reference/tools/files/basic.html#conan-tools-files-save, and in the same way we never documented or used print() in recipes, but instead the self.output is recommended.

The main reason for this is that very often it is necessary to have control and access to certain operations, to apply configuration or other behaviors, and this is only possible if these things are done in recipes via the Conan built-in utilities like save() or self.output.

In any case, I think we can fix it to make it not breaking, thanks very much for your report again.
If this is difficult to workaround for your case, we can try to do a Conan 2.2.2 release instead of having to wait until 2.3.

@memsharded
Copy link
Member

This should fix it: #15934

@memsharded memsharded added this to the 2.2.2 milestone Mar 25, 2024
@sboehmann
Copy link
Author

Many thanks for the quick fix.

@memsharded
Copy link
Member

#17507 adds a new --output-file to enforce writing the result in that file irrespective of the stdout redirections or other possible issues. It will be in next Conan 2.12. Thanks for the feedback!

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 a pull request may close this issue.

2 participants