pdm-project / pdm

A modern Python package and dependency manager supporting the latest PEP standards
https://pdm-project.org
MIT License
7.94k stars 402 forks source link

Output of 'pdm export' gets line wrapped even when the output isn't a terminal #2390

Closed datalogics-kam closed 12 months ago

datalogics-kam commented 1 year ago

Make sure you run commands with -v flag before pasting the output.

Steps to reproduce

In the actual case, we're using subprocess.run(...,stdout=subprocess.PIPE) to consume the output from pdm export. We make a hash and then decide if we want to write some files.

I've been trying to make a small test case, but I haven't figured out what is causing the wrapping behavior. The actual run of PDM is buried in a series of nested subprocesses, and I haven't found anything that would trigger Rich to wrap, like the environment variable COLUMNS being set, or TERM being set to a dumb terminal.

Actual behavior

It seems to work fine most of the time locally, but when running from inside pytest the file gets corrupted: A long line with --extra-index-url with our AWS CodeArtifact URL in it gets line wrapped, and then pip can't use the file.

--extra-index-url 
https://aws:${CIT_PYPI_PASSWORD}@xxxxxxxxxx-xxxxxxxxxxxx.d.codeartifact.us-east-
2.amazonaws.com/pypi/cit-pypi/simple/

Expected behavior

I would expect that in no case would the output of pdm export be formatted in any way. Even if the output is a bit wide for the terminal, it should be consumable by, for instance, pip.

The question I have is whether the export output should be sent through Rich.

Environment Information

# Paste the output of `pdm info && pdm info --env` below:
❯ pdm info && pdm info --env
PDM version:
  2.10.1
Python Interpreter:
  /Users/kam/Desktop/narrow-lines/.venv/bin/python (3.11)
Project Root:
  /Users/kam/Desktop/narrow-lines
Local Packages:

{
  "implementation_name": "cpython",
  "implementation_version": "3.11.2",
  "os_name": "posix",
  "platform_machine": "x86_64",
  "platform_release": "22.6.0",
  "platform_system": "Darwin",
  "platform_version": "Darwin Kernel Version 22.6.0: Fri Sep 15 13:39:52 PDT 2023; root:xnu-8796.141.3.700.8~1/RELEASE_X86_64",
  "python_full_version": "3.11.2",
  "platform_python_implementation": "CPython",
  "python_version": "3.11",
  "sys_platform": "darwin"
}
frostming commented 1 year ago

We support output to file flag --output, why not use that?

pawamoy commented 1 year ago

If using --output doesn't wrap the output, that's great, but it requires more gymnastics to use it (see below), and I guess @datalogics-kam's point is that the output should never be wrapped, whether --output is used or not.

With subprocess, I suppose using --output requires creating a temporary file, then reading again from it once the command has finished.

import os
import subprocess
from tempfile import TemporaryDirectory

with TemporaryDirectory() as tmpdir:
    tmpfile = os.path.join(tmpdir, "requirements.txt")
    subprocess.run(f"pdm export -o {tmpfile}".split())
    with open(tmpfile) as file:
        requirements = file.read()

Using requirements = subprocess.check_output("pdm export") is arguably much easier :slightly_smiling_face:

I'm actually doing the same thing as @datalogics-kam in my project tasks to check my dependencies:

@duty
def check_dependencies(ctx: Context) -> None:
    requirements = ctx.run(
        ["pdm", "export", "-f", "requirements", "--without-hashes"],
        title="Exporting dependencies as requirements",
        allow_overrides=False,
    )

    ctx.run(
        safety.check(requirements),
        title="Checking dependencies",
        command="pdm export -f requirements --without-hashes | safety check --stdin",
    )

Apparently I'm just lucky that none of them use an extra index URL :thinking: I'll verify that to confirm.

pawamoy commented 1 year ago

It seems to work fine most of the time locally, but when running from inside pytest the file gets corrupted

Is it possible that pytest is the culprit here? Without seeing your code and how you test it, it's difficult to guess :thinking:

datalogics-kam commented 1 year ago

Is it possible that pytest is the culprit here? Without seeing your code and how you test it, it's difficult to guess 🤔

It breaks in a situation where there are a lot of layers...coverage calls pytest calls (AWS) cdk synth which is using our custom PythonLayerVersion which wants the requirements from a certain group.

I've tried probing to see what's going on, but my best guess is that something along the way is using a pty or setting environment variables that makes Rich think the output is a terminal that's 80 columns wide.

It's also not real consistent. I tried making a small pyproject.toml that demonstrates the problem, and couldn't get it to consistently break in that case.

For now I did in fact use --output with a temporary file, but conceptually I thought of pdm export as a sophisticated version of pip freeze and just used the stdout without exploring other possibilities.

frostming commented 1 year ago

I'm not opposed to changing to a regular print, can someone send a PR?

pawamoy commented 1 year ago

Erm, I'm so sorry @frostming, I pushed directly to the main branch :scream: I'm not used to having push access on repositories I only contribute to, and GitHub's web interface offered me this choice by default. I clicked too fast. Sorry :bow: Don't hesitate to git reset, and I'll open a proper PR. Also I wouldn't mind that you remove my push access to the main branch :sweat_smile:

pawamoy commented 1 year ago

By the way I can probably do the git reset + force push, but I don't want to make any more mistake, so I'll wait for your input :slightly_smiling_face:

frostming commented 1 year ago

Don't worry about it. I think we should protect the main branch, but sometimes I like to push commits to it, which may not be a good habit.😟

pawamoy commented 1 year ago

I think it's possible to remove push access to the main branch for specific collaborators only, while keeping it for yourself.

frostming commented 1 year ago

@pawamoy done, now you continue with your PR.

pawamoy commented 12 months ago

Thanks!

I'm not sure why but I'm getting 72 Mypy errors when pre-commit runs, preventing me from committing, so I'll just disable the Git hook for now.

datalogics-kam commented 12 months ago

Thanks, @pawamoy, for the fix!!