Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Runfiles should include aliases under the aliased label #18477

Open
katre opened this issue May 23, 2023 · 5 comments
Open

Runfiles should include aliases under the aliased label #18477

katre opened this issue May 23, 2023 · 5 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request

Comments

@katre
Copy link
Member

katre commented May 23, 2023

If I have a target which depends on a label, the runfiles should reference that label, regardless of whether it is an alias (of any depth). However, the current code in SourceManifestAction.writeEntry only sees the list of artifacts with the actual underlying paths, and has lost the original aliased label.

See katre@afe4d9d for a test demonstrating the issue.

@sgowroji sgowroji added type: feature request team-Configurability platforms, toolchains, cquery, select(), config transitions labels May 23, 2023
@gregestren gregestren added the P3 We're not considering working on this, but happy to review a PR. (No assignee) label Nov 14, 2023
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs. If you think this issue is still relevant and should stay open, please post any comment here and the issue will no longer be marked as stale.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 18, 2025
@fmeum
Copy link
Collaborator

fmeum commented Jan 18, 2025

Runfiles manifest entries are for files (potentially multiple per target) and the runfiles returned by a single rule may come from various different on-disk directories, whereas aliases operate on the target level. I don't see what "referencing the alias label" would mean in this context.

@csmulhern
Copy link
Contributor

I believe the idea is you have outputs that are declared relative to some source target, and you want to re-root them relative to the alias target.

For example:

//some/source/path/BUILD.bazel

cc_shared_library(
  name = "foo",
  ...
)

If you have a runtime dependency on this target, then you can retrieve the file using a runfiles library by looking up e.g. rlocation("some/source/path/libfoo.a")

If you aliased this target as e.g.

//aliases/BUILD.bazel

alias(
  name = "foo",
  actual = "//some/source/path:foo"
)

I think the desired behavior would be to re-root those outputs by changing the package prefix of the actual target to that of the alias.

E.g. with the above alias you'd be able to lookup rlocation("aliases/libfoo.a").

A potential use case that made me subscribe to this issue was really around using aliases to select over the same output for multiple platforms, and then hoping to unify the path at runtime.

E.g. imagine if we have two http_archive repos that download a tool, one for linux (@tool_linux_amd64) and one for macOS (@tool_darwin_arm64). Then in our workspace, we have:

alias(
  name = "tool",
  actual = select({
        "//platforms/config:linux_amd64": "@tool_linux_amd64//:tool",
        "//platforms/config:darwin_arm64": "@tool_darwin_arm64//:tool",
        "//conditions:default": ":incompatible",
    }),
    target_compatible_with = select({
        "//platforms/config:linux_amd64": [],
        "//platforms/config:darwin_arm64": [],
        "//conditions:default": ["//platforms:incompatible"],
    }),
  })
)

It'd be nice to be able to query a runfiles library with "path/to/alias/tool" to get the tool path, rather than have to have the binary itself e.g. conditionally compiled to lookup tool from the tool_linux_amd64 repo vs the tool_darwin_arm64 repo.

This also makes sense in the way that a common use of alias is to provide a stable target path while the actual target is moved around. This works for targets in build land, but not for files in runtime land.

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label Jan 19, 2025
@fmeum
Copy link
Collaborator

fmeum commented Jan 19, 2025

Since alias purpose (and unique feature compared to custom rules) is exactly to be as transparent as possible, it seems difficult to find logic that handles this case but is generically useful and correct.

Have you considered making your own custom rule that picks out the executable and symlinks it under its own package's output directory via ctx.actions.symlink? Depending on how skylib's copy_file handles runfiles (I haven't checked), it may even do just that.

With the ability to ask for all Starlark providers to be forwarded unchanged, which has been asked for in other issues, that could even gain parity with alias. Forwarding select providers would work today.

@csmulhern
Copy link
Contributor

I added my own custom alias-esque rule for this type of usage and it's working well. It's a very simple implementation, and I agree that it's not very general purpose, so I think leaving alias as is actually does make sense.

For posterity, I ended up doing something like:

def _alias_executable_impl(ctx):
    output = ctx.actions.declare_file(ctx.attr.name)
    ctx.actions.symlink(output = output, target_file = ctx.executable.actual)
    return [DefaultInfo(files = depset([output]), executable = output)]

alias_executable = rule(
    implementation = _alias_executable_impl,
    executable = True,
    attrs = {
        "actual": attr.label(
            allow_files = True,
            executable = True,
            cfg = "target",
        ),
    },
)

Thanks for the guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants