Genrule recommends using $(location) but it's described as legacy reference to either $(execpath) or $(rootpath)

[hauserx]

Page link:

https://bazel.build/reference/be/general#genrule_args

Problem description (include actual vs expected text, if applicable):

From https://bazel.build/reference/be/make-variables#predefined_label_variables:

location: A synonym for either execpath or rootpath, depending on the attribute being expanded. This is legacy pre-Starlark behavior and not recommended unless you really know what it does for a particular rule. See #2475 for details.

But https://bazel.build/reference/be/general#genrule suggests using $ (location) as well as presents various examples with using it instead of using $(execpath). Multiple examples within bazel codebase also use $ (location). The genrule description links to the make-variables page that contains recommendation not to use $ (location).

Where do you see this issue? (include link to specific section of the page, if applicable)

No response

Any other information you'd like to share?

It would be nice to clarify whether genrule actually should stick to use $ (location) or whether examples and documentation should use $ (execpath) instead.

[gregestren]

That warning might be outdated after 57b3a15, which added short_paths to ctx.expand_location. So the Starlark API has knobs now.

@hvadehra what do you think?

[fmeum]

The API is private though, only former native rulesets can use it.

[gregestren]

Good call.

So location is a conditional synonym of other more precisely defined Make variables. I guess the main doc question is: is that conditional logic important, thus keeping location relevant? Or can execpath or rootpath replace it in all practical places?

[hvadehra]

Looking at

bazel/src/main/java/com/google/devtools/build/lib/analysis/LocationExpander.java

Lines 377 to 424 in 43aa96b

	static ImmutableMap<String, LocationFunction> allLocationFunctions(
	Label root,
	Supplier<Map<Label, Collection<Artifact>>> locationMap,
	boolean execPaths,
	boolean legacyExternalRunfiles) {
	return new ImmutableMap.Builder<String, LocationFunction>()
	.put(
	"location",
	new LocationFunction(
	root,
	locationMap,
	execPaths ? PathType.EXEC : PathType.LOCATION,
	legacyExternalRunfiles,
	EXACTLY_ONE))
	.put(
	"locations",
	new LocationFunction(
	root,
	locationMap,
	execPaths ? PathType.EXEC : PathType.LOCATION,
	legacyExternalRunfiles,
	ALLOW_MULTIPLE))
	.put(
	"rootpath",
	new LocationFunction(
	root, locationMap, PathType.LOCATION, legacyExternalRunfiles, EXACTLY_ONE))
	.put(
	"rootpaths",
	new LocationFunction(
	root, locationMap, PathType.LOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
	.put(
	"execpath",
	new LocationFunction(
	root, locationMap, PathType.EXEC, legacyExternalRunfiles, EXACTLY_ONE))
	.put(
	"execpaths",
	new LocationFunction(
	root, locationMap, PathType.EXEC, legacyExternalRunfiles, ALLOW_MULTIPLE))
	.put(
	"rlocationpath",
	new LocationFunction(
	root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, EXACTLY_ONE))
	.put(
	"rlocationpaths",
	new LocationFunction(
	root, locationMap, PathType.RLOCATION, legacyExternalRunfiles, ALLOW_MULTIPLE))
	.buildOrThrow();
	}

ISTM execpath(s) and rootpath(s) are exactly equivalent to the conditional behavior of location(s), so we just need to update the docs/examples to stop referring to location