pantsbuild #development

Channels

welcome

github-notifications

development

ancient-vegetable-10556

09/16/2022, 4:07 PM

So the current issue is the

FieldDefaultFactoryResult

rules that compute defaults for resolve names. I may be able to inject environments in there, but there’s currently not anything environment-sensitive being used there

ancient-vegetable-10556

09/16/2022, 4:09 PM

oh, there’s also

KnownUserResolveNames

ancient-vegetable-10556

09/16/2022, 4:28 PM

So the temporary solution is to make

EnvironmentsSubsystem, GlobalOptions, GenerateLockfilesSubsystem, JvmSubsystem, PythonSetup

all singletons so I can make the option injection code work, and then come back to how to pick apart the remaining subsystems

👍 1

witty-crayon-22786

09/16/2022, 5:26 PM

imo, both for asthetics and to make this problem easier, we should move to

[GLOBAL] environments = [..]

with these as a bootstrap option.

ancient-vegetable-10556

09/16/2022, 5:28 PM

Point taken, but that doesn’t really help the rest of the issues, some of which need a small amount of design discussion

ancient-vegetable-10556

09/16/2022, 5:28 PM

if it were just

environments =

that were the issue, that would be a nice easy fix

witty-crayon-22786

09/16/2022, 5:30 PM

what makes you think that it is isn’t? it’s not that Options parsing depends on

GenerateLockfilesSubsystem

JvmSubsystem

, etc

ancient-vegetable-10556

09/16/2022, 5:32 PM

for

JvmSubsystem

and

PythonSetup

are dependencies for their respective language’s resolve fields

ancient-vegetable-10556

09/16/2022, 5:33 PM

the option parsing code needs to be invoked in order to figure out the default values for the fields

ancient-vegetable-10556

09/16/2022, 5:34 PM

and to be clear, if this weren’t an unrelated problem, making

EnvironmentsSubsystem

a singleton would have just fixed the issue

witty-crayon-22786

09/16/2022, 5:34 PM

but those fields aren’t relevant to the EnvironmentTarget, are they?

ancient-vegetable-10556

09/16/2022, 5:34 PM

They aren’t yet, but they necessarily are if

PythonSetup

and

JvmSubsystem

remain per-environment

👍 1

witty-crayon-22786

09/16/2022, 5:35 PM

they will consume the Environment during their construction: not the other way around

ancient-vegetable-10556

09/16/2022, 5:35 PM

I understand that

ancient-vegetable-10556

09/16/2022, 5:37 PM

The issue is that we’re defining fields with dynamic defaults that depend on

PythonSetup

and

JvmSubsystem

to be constructed in order to figure out the default values, and currently the field setup side of things is unaware of environments, and it’s not clear that they even should be

ancient-vegetable-10556

09/16/2022, 5:37 PM

(or what it would mean if they were)

ancient-vegetable-10556

09/16/2022, 5:39 PM

so again, this is an unrelated dependency issue to how we interpret

EnvironmentsSubsystem

witty-crayon-22786

09/16/2022, 5:39 PM

got it. and the dynamic defaults code is invoked below EnvironmentTarget construction? should disable that then probably.

ancient-vegetable-10556

09/16/2022, 5:41 PM

Well no, it’s not

witty-crayon-22786

09/16/2022, 5:41 PM

ah… that’s because Eric added that for the docker field. i think that if you nuke that and use the logic directly, you should be good.

ancient-vegetable-10556

09/16/2022, 5:41 PM

It’s not just the docker field

ancient-vegetable-10556

09/16/2022, 5:41 PM

it’s also for PythonSetup and JvmSubsystem, which has existed for aeons

hundreds-father-404

09/16/2022, 5:41 PM

Oh for sure feel free to delete the default field stuff for the docker platform field. it's not strictly necessary

ancient-vegetable-10556

09/16/2022, 5:41 PM

the docker stuff is a red herring

ancient-vegetable-10556

09/16/2022, 5:42 PM

it exists in at least two other places

ancient-vegetable-10556

09/16/2022, 5:43 PM

either way, I am not interested in solving this right now, I’m just telling you that changing the place where

environments

is defined is not going to be a productive change right now

➕ 1

👍 1

witty-crayon-22786

09/16/2022, 5:43 PM

k… Eric will work with you on this.

hundreds-father-404

09/16/2022, 5:44 PM

we could land the current implementation that works, and then try to simplify if possible after

ancient-vegetable-10556

09/16/2022, 5:45 PM

I’ll talk through the main blocker with Eric separately, since it’s mostly their code that’s causing concern 🙂

busy-vase-39202

09/16/2022, 5:45 PM

Reminder: the monthly Pants Team Meetup is this coming Monday! All current members of Pants Team are welcome to join in. If there's anything you'd like to include on the agenda, DM me 👉 _today_👈 to get it added.

👀 1