pantsbuild #development

Channels

welcome

github-notifications

development

witty-crayon-22786

07/15/2020, 1:09 AM

replied

hundreds-father-404

07/16/2020, 12:18 AM

I’m adding the ability to put a file in the

dependencies

field in BUILD files. Pants will find all the owners of the file; if there’s 1 owner, then we will generate a subtarget from that owner which precisely owns that file. Otherwise, we will error. What do y’all think about how to specify paths relative to the BUILD file, rather than relative to the BUILD root? Stu proposed `./`:

Copy code

./relative.txt
//global.txt
demo/foo.txt

🔥 1

aloof-angle-91616

07/16/2020, 12:18 AM

i like ./

aloof-angle-91616

07/16/2020, 12:19 AM

i really like the lack of ambiguity in the above examples

hundreds-father-404

07/16/2020, 12:19 AM

Me too. So then

ambiguous.txt

would be the same thing as

//ambiguous.txt

, I believe. I don’t love that that differs from the

sources

field, but also the

sources

field is different in that you can only refer to a child, not an ancestor

👍 1

aloof-angle-91616

07/16/2020, 12:20 AM

agreed that this seems pretty reasonable for now

💯 1

hundreds-father-404

07/16/2020, 12:20 AM

Awesome. Thank you for the feedback, Danny 🙂

👖 1

❤️ 1

hundreds-father-404

07/16/2020, 12:21 AM

The last remaining change to the

dependencies

field is allowing ignores via

!:ignore_me

aloof-angle-91616

07/16/2020, 12:22 AM

how would that ignore declaration affect the files that are available to import in the target at runtime?

hundreds-father-404

07/16/2020, 12:26 AM

The current proposal is that after we’ve parsed all the

Address

object to include, we will filter out with any excludes. So it solely would impact the result of

await Get(Addresses, DependenciesRequest)

. Unlike

sources

, I think that we will possibly error (maybe warn) if an ignore is not applied. The main motivation is an escape hatch for dep inference, if you don’t like what it inferred.

👌 1

hundreds-father-404

07/16/2020, 12:26 AM

The main motivation is an escape hatch for dep inference, if you don’t like what it inferred.

It’d be goofy, but technically possible, to say

dependencies[":foo", "!:foo"]

👌 1

hundreds-father-404

07/16/2020, 12:27 AM

Ah and to be clear, this solely impacts the resulting

dependencies

for that target. It does not impact anything like transitive dependencies.

aloof-angle-91616

07/16/2020, 12:30 AM

escape hatch for dep inference is super awesome

aloof-angle-91616

07/16/2020, 12:31 AM

the above feels consistent and makes sense to me

💯 1

aloof-angle-91616

07/16/2020, 12:31 AM

could it also impact transitive dependencies?

hundreds-father-404

07/16/2020, 12:32 AM

I don’t think so. That would mean we’re ~~looking~~ leaking this new ignore mechanism to call sites. With this generated subtargets project, a really important goal has been to not leak any of these new mechanisms to downstream rules like

pytest_runner.py

. Rather,

pytest_runner.py

gets a normal looking target right now and has no knowledge that it was generated

aloof-angle-91616

07/16/2020, 12:33 AM

ok so ignores are only applicable for direct dependencies then?

hundreds-father-404

07/16/2020, 12:33 AM

precisely

👍 1

aloof-angle-91616

07/16/2020, 12:35 AM

something like dep banning could use similar syntax in the future

💯 1

hundreds-father-404

07/16/2020, 12:36 AM

That’d be great. The

is meant to align with sources globs

👍 1

hundreds-father-404

07/16/2020, 12:36 AM

which aligns with Git

aloof-angle-91616

07/16/2020, 12:37 AM

as long as we stick to that canonical kind of syntax i think we should be fine. any more metacharacters and i might recommend creating a named BUILD file object instead

hundreds-father-404

07/16/2020, 12:41 AM

The only other symbol in consideration is using

for disambiguating when there are multiple owning targets for a file. For example,

demo/app.txt@:tgt1

vs.

demo/app.txt@:tgt2

. Right now, we avoid that ambiguity ever being an issue, as follows: - file arguments -> don’t use generated subtargets and use all the original owning targets - dep infeerence -> no-op - explicit BUILD files -> error I personally advocate sticking with that and not using

. It depends how much we care about making it ergonomic to have multiple owners over the same target.

aloof-angle-91616

07/16/2020, 12:43 AM

i think the result where we have a pants that doesn’t care about multiple owners because of “smart” dep inference seems like the better result if we can do it

aloof-angle-91616

07/16/2020, 12:44 AM

hmmm. especially in scala i can see that becoming an issue

hundreds-father-404

07/16/2020, 12:44 AM

That would be using something like

to allow for the disambiguation. Notably, users would almost never actually have to write something that clunky. It would get inferred. Only the output of things like

./pants list

would show that new syntax.

hundreds-father-404

07/16/2020, 12:44 AM

Although, I don’t think it will ever be safe for dep inference to disambiguate between multiple owners. We can’t decide which metadata you want based off of an import statement alone.

👍 1

hundreds-father-404

07/16/2020, 12:45 AM

The only place this would help is a) file arguments and b) explicit BUILD files

aloof-angle-91616

07/16/2020, 12:45 AM

@ seems reasonable enough here i was lukewarm on it but the intent seems understandable

👍 1

aloof-angle-91616

07/16/2020, 12:45 AM

and yes that is what i slowly just realized regarding disambiguating by magic

👍 1