pantsbuild #development

Channels

welcome

github-notifications

development

bitter-ability-32190

12/15/2021, 9:07 PM

The

default

for

sources

on this page is malformed: https://www.pantsbuild.org/docs/reference-python_sources#codesourcescode

happy-kitchen-89482

12/15/2021, 9:11 PM

Thanks, this is what happens when we autogenerate markdown from source code

happy-kitchen-89482

12/15/2021, 9:11 PM

Will patch it

🙌 1

bitter-ability-32190

12/15/2021, 9:12 PM

Beats duplicating the information

happy-kitchen-89482

12/15/2021, 9:53 PM

https://github.com/pantsbuild/pants/pull/13891

🙌 2

happy-kitchen-89482

12/16/2021, 2:18 AM

qq re something I noticed in the docker plugin:

happy-kitchen-89482

12/16/2021, 2:18 AM

At this line https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/docker/util_rules/dependencies.py#L31 why do we reference PexBinaryFieldSet specifically instead of the generic BinaryFieldSet ?

happy-kitchen-89482

12/16/2021, 2:18 AM

It seems like a smell to have the docker plugin import from the python plugin

👍 1

hundreds-father-404

12/16/2021, 2:36 AM

Oh I think fixing that would make inference work for go_binary for example?

happy-kitchen-89482

12/16/2021, 3:01 AM

I assume so?

curved-television-6568

12/16/2021, 6:41 AM

What is the naming pattern of a packaged go binary?

hundreds-father-404

12/16/2021, 7:05 AM

It's the same as pex_binary. Bases on the output_path field. Defaults to no file extension

curved-television-6568

12/16/2021, 7:20 AM

So inference will not work right off, as it looks for the .pex extension..

curved-television-6568

12/16/2021, 3:56 PM

It’s an early draft, but feedback welcome on a “Docker build context trouble shooter” https://github.com/pantsbuild/pants/pull/13896

curved-television-6568

12/16/2021, 4:03 PM

Errm.. @happy-kitchen-89482 there is no `BinaryFieldSet`… there is a

PackageFieldSet

, but I can’t use that the way it’s done there now.. as:

Copy code

E           AttributeError: type object 'PackageFieldSet' has no attribute 'required_fields'

curved-television-6568

12/16/2021, 4:04 PM

What I can think of, is to request all field sets applicable for each target.. and use the target if any of those fieldsets inherits from PackageFieldSet…

curved-television-6568

12/16/2021, 4:06 PM

Oh, or maybe just what is already done in https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/docker/util_rules/docker_build_context.py#L206-L209

curved-television-6568

12/16/2021, 4:06 PM

Right, I’ll go with that, I think… 🙂

curved-television-6568

12/16/2021, 4:22 PM

@happy-kitchen-89482 fix https://github.com/pantsbuild/pants/pull/13897 😉

🙌 1

curved-television-6568

12/16/2021, 4:25 PM

@hundreds-father-404 regarding infering for go binaries… this is the relevant section to look at: https://github.com/pantsbuild/pants/blob/main/src/python/pants/backend/docker/subsystems/dockerfile_wrapper_script.py#L16-L37

🙌 1

hundreds-father-404

12/16/2021, 4:28 PM

Idea: capture all paths used when parsing Dockerfile. Then proactively find all

PackageFieldSet

(your new code) and inspect their `output_path`s to determine what file paths can be created. Compare those `output_path`s with the parsed paths, and if any are a match, infer a dep on the binary target I think that will both handle

go_binary

and handle when you override

output_path

on a

pex_binary

target. Wdyt?

curved-television-6568

12/16/2021, 4:29 PM

Sounds great.

curved-television-6568

12/16/2021, 4:29 PM

That also aligns somewhat with the work I’m doing in https://github.com/pantsbuild/pants/pull/13896 where I added just that step with collecting all paths used..

👀 1

hundreds-father-404

12/16/2021, 4:30 PM

capture all paths used when parsing Dockerfile

Not sure how to do that part. It's possible to set

output_path=foo.pex

or even

output_path=foo

. We don't know if that's a path or not? Maybe we don't have to though, we don't aim for 100% coverage. Use a heuristic of if it has a file extension or path seperator (/)

curved-television-6568

12/16/2021, 4:31 PM

I think landing #13896 can be a good stepping stone for further work in this direction.. 🙂

hundreds-father-404

12/16/2021, 4:31 PM

Oh interesting! So there, you determine it's a path based on the

COPY

instruction, right? Rather than parsing all arbitrary strings as possible paths. Brilliant

👍 1

curved-television-6568

12/16/2021, 4:32 PM

Yeah, well, I capture all sources referenced by COPY instructions, which may be paths or files.

🙌 1

curved-television-6568

12/16/2021, 4:32 PM

But it was a good call to go with the

dockerfile

python lib.. it’s been real easy to work with.

❤️ 1

hundreds-father-404

12/16/2021, 4:34 PM

https://github.com/pantsbuild/pants/issues/13898

🙌 1

curved-television-6568

12/16/2021, 4:38 PM

@hundreds-father-404 you assigned yourself to that issue, does it mean you want to look at implementing it?