Synthesized lockfile targets should never err on missing lockfiles.

[kaos]

Fixes #18404

[benjyw]

This seems like a lot of code, to avoid a one-time problem. Why not just not synthesize the target when the source doesn't exist?

[benjyw]

To clarify my comment - we're carving out some new and non-trivial functionality here (namely a target that can confusingly override global glob matching error behavior) just to handle a situation with a synthetic target the user didn't define or know about to begin with.

The purpose of the target is to track deps on the file when it changes, but since it is allowed to not exist, in that case I think it's better - and simpler - to just not synthesize it in the first place.

[cognifloyd]

Can you depend on a synthesized target? Eg, if I wanted to include a lockfile in an sdist, could I depend on the synthesized target? Are there better examples of the utility of the synthesized targets?

Logically, the target does exist, but it has to be synthesized from the contents of pants.toml, not from something defined in a BUILD file. So, I would be surprised if a resolve I explicitly defined (in pants.toml) did not get the synthesized target just because I haven't generated the lockfile yet.

From a different angle: lockfiles are like codegen, but unlike codegen, it gets materialized in the workspace, not under dist/. It's not weird for a codegen thing to not exist. Same with the lockfile. So can we use any of the codegen infra here, allowing things to depend on the file without it existing? But instead of transparently ruining the codegen, it displays a message to run generate-lockfiles.

[kaos]

To clarify my comment - we're carving out some new and non-trivial functionality here (namely a target that can confusingly override global glob matching error behavior) just to handle a situation with a synthetic target the user didn't define or know about to begin with.

The fact that the user didn't know about the target to me contradicts the argument that it would be confusing to not adhere to the global glob matching error behavior.

The purpose of the target is to track deps on the file when it changes, but since it is allowed to not exist, in that case I think it's better - and simpler - to just not synthesize it in the first place.

Although the lockfile is allowed to not exist, that is an intermediate broken state of affairs as we know where it is supposed to exist. I agree that the idea with checking for the existence and not synthesize the _lockfile target in that case would also work, but I don't think that would be any easier. Perhaps this only comes down to from which end we look at this problem, but from my side of the field I see the file missing as a temporary situation that unconditionally must be resolved (eventually) -- and taking this case into consideration when the target is synthesized would spill over to every plugin that synthesizes lockfile targets needing to make extra calls only to check for if the target file in question exists before creating targets. To me that feels like a more complex implementation and in potentially more places going forward, then what we have here.

[kaos]

Can you depend on a synthesized target? Eg, if I wanted to include a lockfile in an sdist, could I depend on the synthesized target? Are there better examples of the utility of the synthesized targets?

Yes, that was the primary motivation to introduce synthetic targets, to be able to depend on the generated lockfiles. Not in order to include them, but for identifying targets for changed since when you have regenerated lockfiles.

pants/src/python/pants/backend/python/macros/common_requirements_rule.py

Lines 77 to 110 in c9def21

	resolve = request.template.get(
	PythonRequirementResolveField.alias, python_setup.default_resolve
	)
	lockfile = (
	python_setup.resolves.get(resolve) if python_setup.enable_synthetic_lockfiles else None
	)
	if lockfile:
	lockfile_address = Address(
	os.path.dirname(lockfile),
	target_name=resolve,
	)
	target_adaptor = await Get(
	TargetAdaptor,
	TargetAdaptorRequest(
	description_of_origin=f"{generator.alias} lockfile dep for the {resolve} resolve",
	address=lockfile_address,
	),
	)
	if target_adaptor.type_alias == "_lockfiles":
	req_deps.append(f"{lockfile}:{resolve}")
	else:
	logger.warning(
	softwrap(
	f"""
	The synthetic lockfile target for {lockfile} is being shadowed by the
	{target_adaptor.type_alias} target {lockfile_address}.
	There will not be any dependency to the lockfile.
	Resolve by either renaming the shadowing target, the resolve {resolve!r} or
	moving the target or the lockfile to another directory.
	"""
	)
	)

Logically, the target does exist, but it has to be synthesized from the contents of pants.toml, not from something defined in a BUILD file. So, I would be surprised if a resolve I explicitly defined (in pants.toml) did not get the synthesized target just because I haven't generated the lockfile yet.

Thank you. Although the connection between the synthesized target and the source file is looser than for codegen, I agree that we have all the pieces to expect the target to exist, also when the actual source file it represents is not there.

From a different angle: lockfiles are like codegen, but unlike codegen, it gets materialized in the workspace, not under dist/. It's not weird for a codegen thing to not exist. Same with the lockfile. So can we use any of the codegen infra here, allowing things to depend on the file without it existing? But instead of transparently ruining the codegen, it displays a message to run generate-lockfiles.

Another difference here is that the synthetic target plays no role in the materialization of the source files, so to me it makes little sense in having codegen involved at all here.

[kaos]

@cognifloyd there's also #17253 for more context (and linked PR #16998)

[benjyw]

I love it - simpler code yet more functionality!

[danxmoran]

@kaos is it possible the changes here have regressed? I'm seeing an error from missing lockfiles on a PANTS_SHA that should include this change (#18933)

[kaos]

Eh... hrrm. It seems to me that this fix never actually worked to begin with 😞 😬

Thanks for discovering @danxmoran -- I'll look into it right away.