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] System requirements pollutes STDOUT #17043

Closed
mtribiere opened this issue Sep 23, 2024 · 5 comments
Closed

[bug] System requirements pollutes STDOUT #17043

mtribiere opened this issue Sep 23, 2024 · 5 comments
Assignees

Comments

@mtribiere
Copy link

mtribiere commented Sep 23, 2024

Describe the bug

Conan version: 2.7.1

While creating a conan package, I noticed that system_requirements() of a downstream package can output in STDOUT of an upstream package, poisoning the json output of conan create . -f json > output.json.

Example of downstream package:

def system_requirements(self):
    subprocess.check_call(
        [sys.executable, 
        "-m", "pip", 
        "install", f"my_launcher=={self.version}"
    ])

Result in the upstream package output.json:

Looking in indexes: https://###/gitea/api/packages/TOOLS/pypi/simple/
Requirement already satisfied: package==0.1.0rc4 in ./.venv/lib/python3.9/site-packages (0.1.0rc4)
Requirement already satisfied: typed-settings==24.4.0 in ./.venv/lib/python3.9/site-packages (from package==0.1.0rc4) (24.4.0)
Requirement already satisfied: pydantic==2.5.2 in ./.venv/lib/python3.9/site-packages (from package==0.1.0rc4) (2.5.2)
Requirement already satisfied: click==8.1.7 in ./.venv/lib/python3.9/site-packages (from package==0.1.0rc4) (8.1.7)
Requirement already satisfied: annotated-types>=0.4.0 in ./.venv/lib/python3.9/site-packages (from pydantic==2.5.2->package==0.1.0rc4) (0.7.0)
Requirement already satisfied: typing-extensions>=4.6.1 in ./.venv/lib/python3.9/site-packages (from pydantic==2.5.2->package==0.1.0rc4) (4.12.2)
Requirement already satisfied: pydantic-core==2.14.5 in ./.venv/lib/python3.9/site-packages (from pydantic==2.5.2->package==0.1.0rc4) (2.14.5)
Requirement already satisfied: tomli>=2 in ./.venv/lib/python3.9/site-packages (from typed-settings==24.4.0->package==0.1.0rc4) (2.0.1)
{
    "graph": {
        "nodes": {
            "0": {
                "ref": "lib_sfcp/1.2.0",
                "id": "0",
                "recipe": "Consumer",
[...]

I'm not sure if that's intended behavior, but it prevents me from parsing the JSON output on all upstream packages that use this package.

How to reproduce it

No response

@AbrilRBS AbrilRBS self-assigned this Sep 23, 2024
@AbrilRBS
Copy link
Member

Hi @mtribiere thanks for your question

This would be expected behaviour, as the check_output call is directly dealing with the stdout/stderr redirection, we have no way to globally control it from Conan.

To fix this, you should change the recipe to follow https://docs.python.org/3/library/subprocess.html#subprocess.check_call recommendations:

To suppress stdout or stderr, supply a value of DEVNULL.

This will silence the stdout/err redirections and fix your issue. Let me know if this helps! :)

@memsharded
Copy link
Member

Furthermore the general recommendation is to use self.run() to run things, not subprocess because Conan can do extra stuff in the invocation like injecting necessary environment from profiles. For package managers we also have custom tools to call Apt() and many others with a proper abstraction.

@mtribiere
Copy link
Author

mtribiere commented Sep 25, 2024

Thank you for your answer, is it a bit of a bumper that any packages could potentially poison the JSON output of a package that would consume it. Especially for us we don't always have the control over our user's package. But I understand that conan doesn't have much control over it.

@memsharded you mentioned a Apt() API, is there any python-pip API available or planned?

@memsharded
Copy link
Member

Thank you for your answer, is it a bit of a bumper that any packages could potentially poison the JSON output of a package that would consume it. Especially for us we don't always have the control over our user's package. But I understand that conan doesn't have much control over it.

Yes, this is inconvenient, but that applies to many other aspects, like a dependency recipe could remove the full Conan cache if they want or have a bug, so at the end of the day using dependencies means they are trusted to be correct and do the right thing in all aspects, including not polluting the stdout output.

@memsharded you mentioned a Apt() API, is there any python-pip API available or planned?

A python-pip API is way more challenging than the others, because Conan itself is running in a Python environment, so installing things there can break Conan. And in general, it is just not possible to install Python packages in the same environment without conflicts. So it would be necessary to have Python virtualenvs created on the fly for them, something like #11601 proposes, but it is quite complicated, we didn't have time to resume working on it.

The recommendation for pip packages would be to install them together with Conan pip install conan xxxx yyyy

@mtribiere
Copy link
Author

Yes, this is inconvenient, but that applies to many other aspects, like a dependency recipe could remove the full Conan cache if they want or have a bug, so at the end of the day using dependencies means they are trusted to be correct and do the right thing in all aspects, including not polluting the stdout output.

You are right, this should be trusted environment, however we have been proven that it wasn't always the case. We might try to implement additional package check on Git level.

So it would be necessary to have Python virtualenvs created on the fly for them, something like #11601 proposes, but it is quite complicated...

That feature is very attractive to us, thought I understand it's complex. I will follow along the development closely.


Thank you for those precious information. I'm closing the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants