pex-tool / pex

A tool for generating .pex (Python EXecutable) files, lock files and venvs.
https://docs.pex-tool.org/
Apache License 2.0
2.52k stars 258 forks source link

Adding support to have all command line flags respected via environment variables #2475

Closed AlexTereshenkov closed 1 month ago

AlexTereshenkov commented 1 month ago

I'd love to see PEX supporting receiving any command line argument by reading a corresponding environment variable. This means that all (or most) command line options listed under pex --help would be having a matching env var. E.g. running

$ pex --seed=verbose --compile

would be equivalent to running

PEX_SEED=verbose PEX_COMPILE=1 pex

PEX does have PEX runtime environment variables, but it only covers some of the flags.

The motivation for this is two fold:

  1. It's quite common from what I've seen for cli tools to make it possible to declare environment variables instead of passing the flags, e.g. https://github.com/buchgr/bazel-remote?tab=readme-ov-file#command-line-flags. This can be done for various purposes such as convenience or security (not exposing the values in the terminal). The pex invocation can become quite lean with most of the complicated logic stored in Shell scripts or elsewhere.

  2. Clients who make calls to Pex do not need to be modified for every new flag that pex expose. Pants is of primary concern.

As an example, in a recent PR, a user added pex_build_extra_args flag option to be able to propagate a PEX command line flag from Pants invocation down the line to pex itself:

    Additional arguments to pass to the `pex` invocation that is used to collect the requirements
    and sources for packaging.
    For example, `pex_build_extra_args=["--exclude=pypi-package-name"]` to force a package called
    `pypi-package-name` isn't included in the artifact.
    Note: Excluding dependencies currently causes Pex to throw an error. You can additionally pass
    the `--ignore-errors` flag.

There was at least one question about this on Pantsbuild Slack.

Not having this ability to pass arbitrary arguments to pex makes it really hard to start taking advantage of new features that come with newer versions of pex. This is because the pex invocations in the Pants codebase are numerous and it's really hard to make sure a particular argument is respected (by keeping the chain of calls correct making sure it does reach the final subprocess spawn).

To avoid going the route of raising individual PRs in Pants trying to add support for random PEX flags user may want to have, it would be so much easier to be able to set the environment variables.


I appreciate pex isn't part of Pants so its development might not be driven by Pants users needs, but I hope that use case (1) is strong enough for this enhancement to be considered.

benjyw commented 1 month ago

Hi @AlexTereshenkov , doesn't Pants's pex_build_extra_args solve this problem there in practice?

AlexTereshenkov commented 1 month ago

Hi @AlexTereshenkov , doesn't Pants's pex_build_extra_args solve this problem there in practice?

Hey Benjy! It would, but only for that particular target. I want the PEX args to be passed on any Pants goal invocation, not just when dealing with that particular target type.https://www.pantsbuild.org/2.22/reference/targets/python_aws_lambda_function

I hope this makes sense?

benjyw commented 1 month ago

This seems easy to add, at least globally, in the pex-cli subsystem, no?

jsirois commented 1 month ago

Thankfully @benjyw stepped in here but @AlexTereshenkov this clearly is a problem for Pants to be handling in its own shed. I happen to know both the Python rule side Process dataclass and the rust side struct have slots for both args and env vars. How is it easier to fill one and not the other? Sounds like a Pants problem to me.

I'll just point out all the things off-putting in this issue filing:

  1. You didn't try too hard to look around: #2242
  2. You give a 2-fold motivation of which your only real motivation is item 2. Always lead with your best argument - never pad arguments with weak bullet points. Its disingenuous.
  3. Your motivation for point 1 is crappy:

    It's quite common from what I've seen for cli tools to make it possible to declare environment variables instead of passing the flag

    This much, fine, but see #2242 - clearly I'm aware of this.

    This can be done for various purposes such as convenience or security (not exposing the values in the terminal).

    Half OK - convenience, sure?, but security - an irrelevant red herring point since PEX currently accepts 0 command line arguments that need or should contain credentials.

    The pex invocation can become quite lean with most of the complicated logic stored in Shell scripts or elsewhere.

    With a shell script you can seal in options just as well as you can env vars so this point makes 0 sense: alias pex 'pex --my-favorite-setting1 foo ...

  4. You point to a PR (https://github.com/pantsbuild/pants/pull/20939) that succeeds at plumbing via args, is approved by another Pants maintainer that set the example themselves with these PRs: https://github.com/pantsbuild/pants/pull/20737 and https://github.com/pantsbuild/pants/pull/20929 and is specifically plumbing --exclude which is already something Pants supports 1st class for, for example, jvm deps where the ecosystem has support for excludes. IOW the PR you point to is exactly an example of a quick hack to work around Pants not supporting deep excludes integration, which, I think Pants maintainers would agree it should.
jsirois commented 1 month ago

I suspect it won't shed favorable light on the OP, but I will note https://pantsbuild.slack.com/archives/C046T6T9U/p1695281564840179 is not accessible to me. I am no longer a member of either the Pantsbuild GitHub org or the Slack. @AlexTereshenkov if you can provide a linen link or copy paste the whole thread you point to, that would be great.

jsirois commented 1 month ago

@AlexTereshenkov I'm going to close this in favor of #2242 since it adds nothing over that issue. I filed that quite a while ago and still haven't gotten to it; so thast should make it clear I'm unlikely to be getting to it any time soon. If you want to pitch in and do the work in Pex to make it so, great. If you'd rather put the work into Pants to make it handle controlling the tools it launches more flexibly and more generally, probably better - more bang for your buck.