Direct references show extra requirements in .txt files

[FlorentJeannot]

I am adding the missing extra requirements for direct references. I think it's better to keep consistency with how the rest of pip-tools works.

Contributor checklist

• [x] Provided the tests for the changes.
• [x] Assure PR title is short, clear, and good to be included in the user-oriented changelog

Maintainer checklist

• [ ] Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
• [ ] Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

[AndydeCleyre]

Thanks!

Yes, the extras should be included. A similar fix is sitting in #1329, not yet reviewed by @atugushev:

    extras = f"[{','.join(xtr for xtr in sorted(ireq.extras))}]" if ireq.extras else ""
    return (
        f"{canonicalize_name(ireq.name)}{extras} @ "
        f"{ireq.link.url_without_fragment}"
        f"{fragment_string(ireq, omit_egg=True)}"
    )

but there I did not normalize the extra names, where you do convert to lower case.

I'd like to know:

• are we getting non-normalized extras names in the first place, or is the extra already normalized elsewhere?
• ~if not already normalized, we should probably run them through canonicalize_name rather than just lower casing, right?~
• ~or is an extra sometimes not just a name, and so would be wrongly mangled this way?~

[AndydeCleyre]

I forgot when I wrote that last comment that extra names are not at all package names, so we do NOT want to use canonicalize names on it.

I guess I'd just like to see that any normalization is needed at this place. FWIW I just tested with #1329 an input file that specified an extra as yAmL and it came out as yaml in the txt. 🤷🏼

[FlorentJeannot]

Hello @AndydeCleyre !

I remember your PR in #1329 but I was thinking, since mine is a small fix, that it could be merged quickly. Though I wished that your PR was merged sooner than later. I like the way you handled the relative path and I think that's a nice feature for pip-tools!

Regarding the normalized names in the extras, I double-checked and InstallRequirement is parsing them in the first place. So I don't think we need to do anything else?

[AndydeCleyre]

Makes sense, and thanks again, as your feedback was central to getting that "right."

I won't close this, but also hope the big one gets merged soon. I will fix it up with your suggested change very soon I think.

[FlorentJeannot]

Sounds good. Also if you have any good starter task to give, let me know and I'll happily contribute to the project.

[atugushev]

@FlorentJeannot According to this discussion https://github.com/jazzband/pip-tools/pull/1453#discussion_r671595549 I thought that we agreed to not include extras once requirements have been complied?

[FlorentJeannot]

@atugushev Sorry, it was not clear to me that my thinking was agreed or if it would be fixed later by @AndydeCleyre's PR (https://github.com/jazzband/pip-tools/pull/1329). I thought some people would be confused and open issues to declare the bug, so I wanted to fix that now.

I guess we have two solutions:

1. we merge this PR to restore the old behavior as a "fix" and later we make a PR which removes all extras in the compiled file
2. we decline this PR, and plan to make another one which removes all extras in the compiled file

Another question: Since @AndydeCleyre's PR contains the fix too, should we remove it there (so we don't bring more confusion by restoring the extras and then removing them)? Or should we make a PR now which removes them, so that @AndydeCleyre can merge this change and tweak his PR?

[AndydeCleyre]

I'm not convinced we should strip extras from the output, FWIW.

The fact that pip freeze doesn't include the extras notation isn't necessarily a positive example, as the whole point of pip tools is that freeze is insufficient and too lossy with information for our needs.

[FlorentJeannot]

@AndydeCleyre While I agree pip-compile should output more information than pip freeze, I'm not convinced it should contain the extras. For example, package A has an [extras] which contains package a and b, and these are also extracted in the .txt file by pip-compile:

A[extras]==2.0
a==1.0
b==1.0

When I do pip install or pip-sync, then pip will then fetch A==2.0 A.a, A.b, a==1.0 and b==1.0. While this is not a big deal because it won't break the installation and install the correct version of a and b, it seems a bit redundant.

Also what if package A is compromised?

Let's say package A is updated and [extras] now contains a, b and malware. Then let's say I don't run pip-compile -U to update my .txt but I do pip-sync because I want to refresh the list of packages I've already pinned. pip will fetch A==2.0 A.a, A.b, A.malware, a==1.0 and b==1.0. So I think in this case we would install a package without realizing it, no?

[AndydeCleyre]

I honestly don't understand the example but want to point out in this discussion that some packages use extras to describe dependencies that are then detected during installation, affecting the installation procedure.

If the relevant deps are just present in reqs.txt, installing with pip install -r reqs.txt will not guarantee installation order, which can break the req[extra] installation which needs the extras installed beforehand. Keeping them there, I think, guarantees the necessary order.

EDIT: See #992 for some relevant discussion

[taleinat]

It would be great to have this, or https://github.com/jazzband/pip-tools/pull/1329, or any other solution for extras soon.

I currently am having issues using pip-compile in a project requiring Celery with the "sqs" extra, because "Celery[sqs]" requires "kombu[sqs]" but pip-compile keeps only "kombu" for the sub-dependency :(

[ssbarnea]

@atugushev What is your opinion on this one?

The way extras work (or not) makes me want to discourage their use, especially in result of pip-compile. Does everyone know that you can feed any typo to as an extra and pip will not complain? This proved to be a permanent source of problems as projects add/remove/rename extras over time.

I also have to confess that I am only using pre-commit to build constraints files, so I never want to see extras in them as the spec forbids them. That means that I should let others decide.

[taleinat]

FWIW, for my use case (properly installing "Celery[sqs]"), it would be fine if the output of pip-compile would include all of the required sub-dependencies without the extras.

[atugushev]

@ssbarnea I summed up my opinion here.