https://pantsbuild.org/ logo
Join Slack
Powered by
Hey! I've got a bit of an issue with trying to ge...
# general
g
Hey! I've got a bit of an issue with trying to get 
console_scripts
to work in a 
pex_binary
that I'm not sure how to solve. Essentially I've got a package that has two entry_points; * 
server
* 
worker
server is the main entrypoint, and spawns workers when requested via GRPC. I can set the server as the main entrypoint in a pex-binary, and that works fine. However; the console_script for the worker is never available. I've also tried this: https://github.com/pantsbuild/pants/discussions/17308, but while it shows the entrypoints they aren't on the path. I would expect this to work if I could install the packages into the pex; but it doesn't quite work either due to how the install happens. Our source layout is
Copy code
src/
  py/
    a/
      pyproject.toml
      README.md
      a/
       ..
    b/
      pyproject.toml
      README.md
      b/
       ...
With 
py/a
, 
py/b
being individual source roots (
marker_filenames = ["pyproject.toml"]
). However; when depending on 
a:package
and 
b:package
the pex cannot be built due to the README.md colliding.
Copy code
Exception: Can only merge Directories with no duplicates, but found 2 duplicate entries in :

`README.md`: 1.) file digest=d31d7752e98a52694aaa502b3ce9e49ef02aeb2459a4dcad81eba6456567d077 size=855:
...
`README.md`: 2.) file digest=964e4cf6af0fa0dfe069e7cd383792d591eb94ad56d41bd16aad4f72f36d6816 size=3501:
...
Which makes sense. But I don't think there's anything wrong with our setup either. Any thoughts?
✅ 1
e
Instead of depending on a 
python_distribution
just to get console script entry point metadata, you can write down entry point metadata: https://github.com/pantsbuild/scie-pants/blob/main/tools/BUILD If you also publish a wheel for some reason this will probably be unappealing since it is non DRY.
I use that setup exactly because I have >1 internal console scripts I want to subprocess call to.
g
I do have that; unless there's some difference between your approach and what's in the discussion link?
e
Can you paste your 
pex_binary
target and describe the target type of each of it's dependencies?
I depend on 0 
python_distribution
in mine.
My assumption here is that the only thing that should be pulling in a README is a 
python_distribution
target.
If not, don't glob README!
g
Sure. This is what I do
Copy code
pex_binary(
    name="testing",
    entry_point="server.py",
    dependencies=[
        "//src/py:mock-dist-info",
        "//src/py/server-plugins:package",
     ]
)
The README issue is only if I try to depend on multiple packages.
If I don't, it just doesn't find the console scripts.
e
The last dep sounds an awful lot like 
python_distribution
targets. Are they?
g
It is. It's a plugin for the main server.py.
e
That's what you do not want
g
I do. 🙂 But does that interfere with console_scripts? Consider it to be 
b
above, unrelated to server/worker (both in 
a
)
e
Ok, let me back up since there are 2 things here.
To get console scripts on the PATH you must use an 
execution_mode="venv"
pex_binary
1st and foremost.
g
Ah!
e
As to the dup error, there are a few ways to solve.
My example from scie-pants is one: do not use a wheel middleman in your own repo. As Benjy pointed out in the discussion, it uses a literal .dist-info dir on disk with just console script entry point metadata in it and a 
python_resources
target to own it. 0 
python_distribution
targets in sight.
g
Yeah; that was just me trying to get it to work.
e
Another way is to avoid the console script middleman too! (I could not do this in my example for reasons). Just 
sys.executable -mthis
In a subprocess
g
So now I've got
Copy code
pex_binary(
    name="dummy",
    entry_point="erupt.entry_points.server:main",
    dependencies=[
        "//src/py:mock-dist-info",
        "//src/py/erupt-gym:package",
    ],
    execution_mode="venv",
)
and
Copy code
✦5 ❯ cat src/py/anyname.dist-info/entry_points.txt
[console_scripts]
erupt-runner = erupt.entry_points.runner:main
erupt-server = erupt.entry_points.server:main
But subprocess fails with
Copy code
FileNotFoundError: [Errno 2] No such file or directory: 'erupt-runner'
e
So the real error message includes a venv path with a sha256 path component in it presumably? Can you go there and inspect the venv? What does bin/ have in it? What about site-packages? Clearly something is wrong and you should be able to spot it.
g
So neither the 
anyname.dist-info
folder nor the 
erupt
are in that site-packages directory, nor can I find the binaries in the bin directory. Is there a difference between 2.14 and 2.15 here?
e
No, venv support is quite old. Something more basic in BUILDs is wrong.
Stepping further back, how do you run this?
g
pants run cmd:dummy -- some_args
e
Ok. There is a middleman there we could take out as a debug step, but I'll save that tactic for now.
So can you reveal the contents of the mock dist info BUILD?
g
Copy code
$ cat src/py/BUILD
resources(name="mock-dist-info", sources=[
    "erupt.dist-info/**/*",
    "erupt.dist-info/*"
])
I changed the name from 
anyname.dist-info
to 
erupt.dist-info
just to ensure it wasn't some weird thing happening there with it not belonging to a package.
(but since erupt isn't in the venv, it's a bit moot.)
e
And what about the BUILD for the other target?
g
The second dependency? That one is in the .venv as intended
Copy code
resources(name="package_data", sources=["pyproject.toml"])

python_distribution(
    name="package",
    dependencies=[
        ":package_data",
        "./erupt_gym:erupt_gym",
    ],
    provides=python_artifact(
        name="erupt-gym",
        version="0.1.0",
        long_description_content_type="markdown",
    ),
    entry_points={
        "erupt.engines": {"Gym": "erupt_gym.engine:GymEngine"},
    },
    long_description_path="src/py/erupt/README.md",
)
e
So don't depend on the middleman here, depend on it's ./erupt_gym:erupt_gym only directly here instead
g
I can remove the dependency altogether, I don't need the plugin at this point in time.
e
You need at least 1 entry point + 1 console script to debug the mechanism IIUC.
g
I've got two entry points from the main package (
erupt
), 
runner
and 
server
.
So server is launching a sibling command in the same package.
e
Gotcha. So why do you want the console script middleman man vs sys.executable -m ...?
g
Want I think is a strong word. It feels a bit more decoupled and is less noisy to read.
e
Ok, that's fine. It's more complicated to debug, but just wanted to confirm 1st you had a reason.
g
Sure. I'm noticing that sometimes the crash happens for code running /tmp/pants-sandbox-xxxxx/ and sometimes the main package code seems to be in the venv.
e
So, back to the original 
pex_binary
target, it should now only have dependencies on just the dist info resource and the server / runner code. Is that right?
g
Yes. Just to double-check I've added an explicit dependency on the runner too:
Copy code
pex_binary(
    name="dummy",
    entry_point="erupt.entry_points.server:main",
    dependencies=[
        "//src/py:mock-dist-info",
        "//src/py/erupt/erupt/entry_points/runner.py"
    ],
    execution_mode="venv",
)
e
Ok, 2 things: 1. checking the venv structure again would be good, the site-packages dir should have all the expected things or dead in the water 2. perhaps stepping back to your original state with just changes of venv mode and drop the long_description to work around README collision 1st
Ok, that's fine. It's more complicated to debug ...
I will say that, if we get this working, it is definitely alot of ceremony, files and indirection to avoid 
sys.executable -m ...
which any new hire with Python experience could read, grok, and find what is being invoked.
g
OK; so. Did you see my point about the code sometimes running in sandbox and sometimes in ~/.pex/? Because when it's in ~/.pex both erupt and the dist-info are in the site-packages. Nothing in bin, unfortunately.
e
Ok, lets remove 
./pants run ...
and instead just 
./pants package ... && ./dist/my.pex
- I'm pulling in the complication removal card.
1 thing at a time. What do you get from the latter run technique.
And let me know if you're on tack 1 or 2 from my 2 things list above.
g
On 1 - I'd have to go through and edit all our local packages, add them as package-deps, and remove the README so I'm waiting as long as possible with that.
Same error with pants package.
e
And can you perform the venv inspection step? What is in there? Does site-packages have the code and dist-info or not?
g
image.png,image.png
e
So, site-packages good, bin bad - is that right?
g
Yepp!
e
Ok, so my a apologies in advance and a step back again: Adding the mock dist-info gets you importlib hooked up: https://docs.python.org/3/library/importlib.metadata.html#entry-points But it does not cause console scripts wrapping use of that in bin to be generated. You'll have to look up the real entrypoint using the importlib API. On the step back, do you still feel it makes sense to use this middle-man arrangement? It totally makes sense to support 3rdparty plugins but much less so on an internal codebase - depending on factors.
Is this just Poetry legacy IOW?
g
It's legacy, multi-repo from times long gone. Predates Poetry being a thing 😛
e
Ok.
SO all internal though? I.E.: The README being packaged in a wheel will help no one on PyPI since it doesn't get pulished there and local folks can just read README in the repo?
g
It'd get published to artifact-registry, but all internal there. Yes. If 
sys.executable
does work I can just use that, right? Should side-step the whole thing. And console_scripts would still work for installing packaged wheels.
e
Yes, sys.executable is definitely the simplest thing by far if you're amenable. It cuts through alot of layers.
g
Yeah; I'll switch. Just some refactoring to handle some debug workflows I think. And then I guess that means the README thing can be ignored; though a bit of an annoyance.
e
So, back to that. Pants is annoying here because Pex supports yoloing on collisions:
Copy code
usage: pex-tools PATH venv [-h] [--scope {all,deps,srcs}] [-b {false,prepend,append}] [-f] [--collisions-ok] [-p] [--copies] [--compile] [--prompt PROMPT] [--rm {pex,all}]
                           [--non-hermetic-scripts] [--emit-warnings] [--pex-root PEX_ROOT] [--disable-cache] [--cache-dir CACHE_DIR] [--tmpdir TMPDIR] [--rcfile RC_FILE]
                           PATH

Creates a venv from the PEX file.

positional arguments:
  PATH                  The directory to create the virtual environment in.

options:
...
  --collisions-ok       Don't error if population of the venv encounters distributions in the PEX file with colliding files, just emit a warning. (default: False)
IOW you should be able to re-wind to your original BUILD, add venv mode, then 
./pants package ... && PEX_TOOLS=1 ./dist/my.pex venv --collisions-ok right/here && ./right/here/pex
As a long-winded way to run the thing and only get warnings on collisions.
But also stepping so far back we just fell off the edge, the warning is saying something very real. Real packages out on PyPI do exactly this and include files in the top-level namespace like READMEs - not good!
An sdist can / should include a README - a wheel never
g
Hmm, interesting. So I wonder if that is something I can deal with in PDM then. I don't care for having them in the wheels.
e
Yeah, very bad there. It would be great if you can limit to sdists.
After all, you install 3 of your wheels using pip in a venv and you get a silent random solitary README
PDM uses a different thing than venvs and does not have this issue IIUC, but it is an odd duck pushing a PEP there.
Looks cool, but Frost is the only one implementing that PEP so far I think.
Oh, it has the same problem I think. Still a single site-packages dir, just in a different spot: https://peps.python.org/pep-0582/
So you still yolo and get a random README winning unless PDM is pedantic like Pex and warns or errors.
So, in short, I'd be surprised if PDM / Frost pushed you in the direction of including READMEs in wheels.
The Frost connection is this fwiw: https://frostming.com/2021/01-22/introducing-pdm/
g
Oh yeah, I contribute a bit to PDM 🙂 I'm well aware. Though pure PDM builds; nor 
pants package
show the README being included.
e
Aha - Ok! Forgive my ignorance.
Interesting, so you only see this for 
./pants run ...
?
g
Yepp! I think what might be happening is that the README gets bundled as a transitive dependency.
e
Ok, this was all very very long winded because of me not getting what was going on here. Sounds like a bug to me! 
./pants run ...
and `./pants package ... && ./my.pex`should behave the same. Any deviation is alomst certainly a bug.
Do you mind filing an issue? You might want to check 2.15.x 1st before doing so though. There has been alot of motion in Python 
run
machinery in the last few releases and I've lost track a bit of what works like what when.
g
Sure thing! I'll see if I can isolate a repro with code I can share publicly. 🙂
e
Excellent. Thanks for hanging in there with me and dragging me along to clarity. I think I just shoveled mud in your direction for all that effort.
g
I'm sure I could've explained a bit better too 🙂 Thanks a lot for your help!
I was just reading through this again @enough-analyst-54434, and just to be clear packaging the pex and running the pex behave the same (README duplication). The difference is that the README doesn't even exist in the individual wheels, so whatever bugs out isn't because of the packages themselves being incorrectly built. https://github.com/tgolsson/pants-pex-readme-collision-repro
Copy code
❯ pants package src::
10:52:56.64 [INFO] Wrote dist/a-project-0.1.0.tar.gz
10:52:56.64 [INFO] Wrote dist/a_project-0.1.0-py3-none-any.whl
10:52:56.64 [INFO] Wrote dist/b-project-0.1.0.tar.gz
10:52:56.64 [INFO] Wrote dist/b_project-0.1.0-py3-none-any.whl

❯ zipinfo dist/a_project-0.1.0-py3-none-any.whl
Archive:  dist/a_project-0.1.0-py3-none-any.whl
Zip file size: 1206 bytes, number of entries: 5
-rw-r--r--  2.0 unx        9 b- defN 16-Jan-01 00:00 a/__init__.py
-rw-r--r--  2.0 unx       33 b- defN 16-Jan-01 00:00 a/aardvark.py
?rw-------  2.0 unx       87 b- defN 16-Jan-01 00:00 a_project-0.1.0.dist-info/WHEEL
?rw-------  2.0 unx      224 b- defN 16-Jan-01 00:00 a_project-0.1.0.dist-info/METADATA
?rw-------  2.0 unx      346 b- defN 16-Jan-01 00:00 a_project-0.1.0.dist-info/RECORD
5 files, 699 bytes uncompressed, 558 bytes compressed:  20.2%

❯ zipinfo dist/b_project-0.1.0-py3-none-any.whl
Archive:  dist/b_project-0.1.0-py3-none-any.whl
Zip file size: 1203 bytes, number of entries: 5
-rw-r--r--  2.0 unx        9 b- defN 16-Jan-01 00:00 b/__init__.py
-rw-r--r--  2.0 unx       33 b- defN 16-Jan-01 00:00 b/bowling.py
?rw-------  2.0 unx       87 b- defN 16-Jan-01 00:00 b_project-0.1.0.dist-info/WHEEL
?rw-------  2.0 unx      224 b- defN 16-Jan-01 00:00 b_project-0.1.0.dist-info/METADATA
?rw-------  2.0 unx      345 b- defN 16-Jan-01 00:00 b_project-0.1.0.dist-info/RECORD
5 files, 698 bytes uncompressed, 557 bytes compressed:  20.2%

❯ pants package cmd::
10:53:15.75 [ERROR] 1 Exception encountered:

Engine traceback:
  in `package` goal

Exception: Can only merge Directories with no duplicates, but found 2 duplicate entries in :

`README.md`: 1.) file digest=96d7f2c61ad10f92e449136b94433c482ad88ef52f8d9513dd1426c4fb3ea17e size=44:

# B's README

Dummy placeholder lorem ipsum


`README.md`: 2.) file digest=028373478b36dc388b32c231aacb2962b6530618c38ae32af9aad1e7ace29323 size=44:

# A's README

Dummy placeholder lorem ipsum
e
What happens, though, when you package the app (Pex)? Also a zip file that you can inspect the same way.
g
packaging the pex and running the pex behave the same (README duplication)
So we never get that far.
e
I thought you did! 
./pants run
vs 
./pants package && ./dist/my.pex
I was trying to nudge you there as a debugging step anyhow. Remove Pants from the equation basically.
g
That wasn't using the packages, as I mentioned yesterday. It'd be quite a few internal packages to change the BUILD + pyprojects which I wanted to avoid unless absolutely necessary
e
But building the PEX will be - maybe - an instructive step even if it fails to run. I'm pretty lost I think with the state of the remote debugging here at this point. I'd need a crisp re-cap with directory structures, BUILD file contents and command lines to look at later in my morning if you want me to dig further.
g
That's why I put up the repo above 😉 It's a clean repo, with directly using the packages. https://github.com/tgolsson/pants-pex-readme-collision-repro
e
Great, thanks. It appears to be missing a readme. What exact commands should I run to get errors. I'll 1st repro then investigate in the (later) am.
g
Added a README 🙂
e
Aha - it is not a PEX collision, its a Pants engine collision. Super confusing. Digging further...
So, the issue is here: https://github.com/pantsbuild/pants/blob/f862de125add9bf6fc9aa303bc9d168e2f43833b/src/python/pants/backend/python/util_rules/local_dists.py#L1[…]221 When you depend on a wheel, we add the wheel, but we also add back any sources the wheel does not have int it that the wheel target (`python_distribution`that defines the wheel) owns. Looking at why we do that, but that's what kills you here.
g
Yeah; that seems unwanted. Something-something resources? But those should be embedded into the wheel at that point...
e
Yeah, still digging. The growth of the ability to depend on a wheel has been organic and not thought through from the beginning as a whole. As such it has quirks - ne bugs for certain use cases. I'll report back either way, but likely file an issue.
Ok, it looks like this behavior went in from day 1 with no comment. I added a comment: https://github.com/pantsbuild/pants/pull/12573/files#r1106209270 @happy-kitchen-89482 do you remember the motivation for including non-wheel content from 
python_distribution
target when depending on one of these in some other target?
This leads to an issue where README files in many 
python_distribution
targets - which do not make it in the wheel built from it - do make it into a LocalDists PEX usages in leaf rules and thus verbs acting on the upstream targets depending on the 
python_distribution
. This tanks the engine with 2 different digests for same file - README.
Alright, Benjy provided some insight. I've filed an issue to track this bug, but I'm not sure what the solution will be: https://github.com/pantsbuild/pants/issues/18254 Thanks again for the repro repo @gorgeous-winter-99296.
g
Thanks for all the help! I guess we (as in, my team at Embark) need to redesign some workflows around the limitations (and opportunities!) of pex's. They're clearly not amenable to our "incremental"/frameworky way of building software. 🙂
e
@gorgeous-winter-99296 just to double underscore and hopefully make clear - I was wrong when I suggested this was a Pex issue. It is not. Pex confusingly also has collision handling code. That code can be told to warn, but defaults to error. This issue you hit is purely up in Pants. It also has collision detection code and it's that code that is tripping. You cannot turn that error off.
You'd presumably hit this same error in Bazel or any other build tool that uses sandboxing.
The problem here was just that 95% of my support work is Pex, I am the Pex maintainer, and I assume my code is broken. Took me a while here to see that was not true and I confused you as a result.
g
Yeah, I understand. I think the behavior makes sense; though changing it would definitely be better for my specific use-case. Which, after thinking a bit, is probably wrong. My whole reason for doing plugins/framework/entrypoints (and thus having all these problems) is to do as small rebuilds of container images as possible because ML (torch + cuda) containers are slow as heck to build. It's not a business need; just a CI/infra optimization. And now I'm trying to make Pants use that optimization to avoid rewriting at most a couple thousand LOC - when Pants already has excellent caching. So I'll just bite that bullet now and I won't need need entry_points or anything like that, realistically.
e
Gotcha. Well, it's also true there is a real Pants bug here regardless of your refactor away from the trigger for it. It still lurks; so hopefully we'll find time to clean that up soon. Depending on `python_distribution`targets needs some love to clean up this corner as well as a few others.
h
Just caught up on this thread. Sorry for the trouble! The underlying conceptual problem is this: When 
python_library
A depends on 
python_distribution
B, we take that to mean "build the wheel for B and have A consume that". But - when we compute the transitive dependency closure of A, we naively follow the edge to B the same as we would through any other target. So now we have a single big pile of files in the closure, and we subtract out the ones that are in B's wheel, because we know B provides them. We can't subtract out any other files we reached via B because A might need them (via some other dep path). So I think what we really should be doing is not following 
python_distribution
B's deps during the dep graph traverse. We should treat everything behind it as provided by it. So if some file is still in the closure we know we reached it some other way, but not by traversing through B.
And this is probably generalizable to packageable targets in general - if we take "source target depends on packageable target" to mean "build the package and depend on it that way" then I think the above holds.
This is likely not a small change.
And it introduces a new concept
But I think it is more correct
e
I think the easiest way to reason about it is a totally alternate impl which I think was rejected on perf grounds, but is clarifying. For wheels we could build them and then add them to a find links and then resolve and then build PEX. I think that alternate impl provides clarity on delineation even if not feasible.
Same reasoning presumably works for the JVM version of this problem.
h
I don't see how that would solve this? We'd still be pulling in transitive deps through the wheel's target.
Those two READMEs would still show up
e
Ok. It's clarifying to me, it says in a simple way that B is a blob the resolve system should handle. Don't traverse.
Just like any other 3rdparty dep.
h
Yes, so I think we agree on that traversal part. Your suggestion above is a difference in how we make the blob available, and may be a better solution than what we do today.
7 Views
Open in Slack
PreviousNext
Powered by