Weird issue with the solver (thread)
# development
h
Weird issue with the solver (thread)
Copy code
Too many sources of dependency Get(PythonExecutable, [InterpreterConstraints]) for @rule(pants.backend.python.util_rules.pex:428:_determine_pex_python_and_platforms(PexRequest) -> _BuildPexPythonSetup, gets=[Get(PythonExecutable, [InterpreterConstraints])]) <1>: [
        "@rule(pants.backend.python.util_rules.pex:338:find_interpreter(InterpreterConstraints, PexSubsystem, EnvironmentTarget, UnionMembership) -> PythonExecutable, gets=[Get(ProcessResult, [PexCliProcess])]) (for (EnvironmentName, InterpreterConstraints))",
        "@rule(pants.backend.python.util_rules.pex:338:find_interpreter(InterpreterConstraints, PexSubsystem, EnvironmentTarget, UnionMembership) -> PythonExecutable, gets=[Get(ProcessResult, [PexCliProcess])]) (for (EnvironmentName, InterpreterConstraints))",
        "@rule(pants.backend.python.util_rules.pex:338:find_interpreter(InterpreterConstraints, PexSubsystem, EnvironmentTarget, UnionMembership) -> PythonExecutable, gets=[Get(ProcessResult, [PexCliProcess])]) (for (EnvironmentName, InterpreterConstraints))",
    ]
Note that these three sources are the same rule
And this happens when I add a call-by-name in an unrelated file:
Copy code
setuptools_scm_pex = await create_venv_pex(
        **implicitly({
            setuptools_scm.to_pex_request(): PexRequest,
            pex_env: PexEnvironment,
        }),
    )
And weirdly, the behavior is not entirely consistent, sometimes I get two such errors
Replacing the call-by-name with an old style
Get
seems to make this go away
cc @wide-midnight-78598 in case you’ve seen something like this before, and @witty-crayon-22786 in case this makes sense to you
Oh, I lied, the
Get
only solves things if I get rid of the
PexEnvironment
argument, if I add it in using multi-param, it fails with the same error.
And I just verified that call-by-name without the PexEnvironment arg works.
So this is specifically to do with that extra param
w
Does this work?
Copy code
setuptools_scm_pex = await create_venv_pex(
        {
            setuptools_scm.to_pex_request(): PexRequest,
            pex_env: PexEnvironment,
        },**implicitly()
    )
I need to spend more time on the multi-arg use case, which is kinda rare, but needs to be dealt with. because, like....... can't you just pass them as explicit args?
errr... positional args
h
No, that gives an unhashable dict error
I thought
**implicitly
was the way to do multi-param?
In any case, the PexRequest arg is not of the exact type expected by create_venv_pex(). That expects
VenvPexRequest
, and there is a rule that converts them.
w
I thought
**implicitly
was the way to do multi-param?
The use case of converting Get(dict) to call-by-name has always seemed strange to me, I think what you're doing is correct, but it also looks weird - as I assumed most of the dict calls could be resolved into straight parameters
But yeah, the PexRequest problem again
And you have the first and 3rd args, but not the middle, otherwise I was thinking something like
Copy code
**implicitly(setuptools_scm.to_pex_request()), pex_env)
w
If you can pass positional (or keyword) arguments directly to the function (because they are already the right type), then do that. The exceptions are when you actually need arguments to be used to compute other implicit arguments.
for example: if the
PexRequest
conversion requires a
PexEnvironment
(and doesn't already have one in scope), then you need both of those as arguments to
implicitly
w
What would be the pattern for:
Copy code
async def create_venv_pex(
    request: VenvPexRequest, bash: BashBinary, pex_environment: PexEnvironment
) -> VenvPex:
when you to convert the request, and don't have the BashBinary? Apparently this isn't working:
Copy code
await create_venv_pex(
        **implicitly({
            setuptools_scm.to_pex_request(): PexRequest,
            pex_env: PexEnvironment,
        }),
    )
h
Yeah, the
PexRequest -> VenvPexRequest
conversion does require a
PexEnvironment
So I would have expected this to work:
Copy code
await create_venv_pex(
        **implicitly({
            setuptools_scm.to_pex_request(): PexRequest,
            pex_env: PexEnvironment,
        }),
    )
BTW @wide-midnight-78598 There is a TODO here about supporting keyword arguments to the rule in call-by-name
w
👍
Not sure if you've had a chance to check, but do you know how much time is taken by the rule visitor in parsing all the rules? And how frequently that happens? In the migration, it's like in the order of a second or two or something like that for most of my calls, but that happens so infrequently - 1 second on each pants run would be a lot
h
I haven’t measured parsing specifically, but based on my debugging it feels like a few seconds at least
I am still completely at a loss about this issue
For example I can generate this error:
Copy code
Too many sources of dependency pants.backend.python.util_rules.pex.create_venv_pex(, **implicitly(PexEnvironment, PexRequest)) -> VenvPex for @rule(pants.backend.python.util_rules.vcs_versioning:60:generate_python_from_setuptools_scm(GeneratePythonFromSetuptoolsSCMRequest, SetuptoolsSCM, ChosenLocalEnvironmentName) -> GeneratedSources, gets=[pants.vcs.git.get_git_worktree(<1>, , **implicitly(EnvironmentName)) -> MaybeGitWorktree, pants.core.util_rules.adhoc_binaries.get_python_for_scripts(, **implicitly(EnvironmentName)) -> PythonBuildStandaloneBinary, pants.backend.python.util_rules.pex_environment.find_pex_python(, **implicitly(PythonBuildStandaloneBinary)) -> PexEnvironment, Get(builtins.Digest, [CreateDigest]), pants.backend.python.util_rules.pex.create_venv_pex(, **implicitly(PexEnvironment, PexRequest)) -> VenvPex, pants.backend.python.util_rules.pex.setup_venv_pex_process(, **implicitly(PexEnvironment, VenvPexProcess)) -> Process, pants.engine.process.fallible_to_exec_result_or_raise(, **implicitly(Process, EnvironmentName)) -> ProcessResult, Get(builtins.Snapshot, [CreateDigest])]): [
        "@rule(pants.backend.python.util_rules.pex:1042:create_venv_pex(VenvPexRequest, BashBinary, PexEnvironment) -> VenvPex, gets=[pants.backend.python.util_rules.pex.build_pex(<1>, , ) -> BuildPexResult, Get(builtins.Digest, [CreateDigest]), Get(builtins.Digest, [builtins.MergeDigests])]) (for (EnvironmentName, PexEnvironment, PexRequest))",
        "@rule(pants.backend.python.util_rules.pex:1042:create_venv_pex(VenvPexRequest, BashBinary, PexEnvironment) -> VenvPex, gets=[pants.backend.python.util_rules.pex.build_pex(<1>, , ) -> BuildPexResult, Get(builtins.Digest, [CreateDigest]), Get(builtins.Digest, [builtins.MergeDigests])]) (for (EnvironmentName, PexEnvironment, PexRequest))",
    ]
Even when calling that dependency by name!
In which case why is that call even participating in solving
I guess because we haven’t gotten around to short-circuiting that
But as you can see, the two supposedly conflicting sources of the dependency are identical
w
And just to be clear - this sources of said dependency are exclusively from the rule visitor? Or is that combined with info elsewhere? Like, is it possible the PexEnvironment has a different hash or something? So like "the same" but not?
h
The rule visitor provides just one dependency, it gets split into two during monomorphization, for reasons I am not yet sure about, and then not re-joined
w
Ah okay - that would explain why I didn't see it while reviewing code. Is this issue you're seeing already in a branch somewhere?
h
If I print the relevant graph edges before monomorphization, there is just one, after there are two
Yes, I reproduce this consistently
w
Yes, I reproduce this consistently
Right, sorry, I meant like... where? Is there something I can clone or run on my machine to repro the same?
h
Ah, I will push something
👍 1
Just run
pants
in that branch to see the errors
Commenting out line 126 makes the solver pass.
I added a bunch of debugging prints inside builder.rs and have established that before
monomorphize()
there is only one relevant edge, but after it there are multiple
w
👍 Will try to take a look tomorrow
h
Any luck with this?
w
Looked at it, no solution yet.