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

How can you get a library's output path from bxl? #528

Open
zjturner opened this issue Dec 19, 2023 · 7 comments
Open

How can you get a library's output path from bxl? #528

zjturner opened this issue Dec 19, 2023 · 7 comments

Comments

@zjturner
Copy link
Contributor

zjturner commented Dec 19, 2023

I want the equivalent of buck2 targets root//:foo --show-output but implemented from bxl.

Things I tried:

Assume that in all of the following cases, I have output = ctx.analysis(target).providers()[DefaultInfo].default_outputs[0]

  1. output: This gives is an artifact object, but there appears to be no way (that I can find anyway) to get the path of that artifact in the output directory.

  2. ctx.output.ensure(output): That returns an ensured_artifact which have abs_path() and rel_path() methods, but those methods dont' return strings, they just appear to return the original ensured_artifact. Besides, I don't actually want to build them, I just want their paths (similar to what buck2 targets --show-output does).

  3. ctx.fs.abs_path_unsafe(output). I get the unusual error:

Traceback (most recent call last):
File , in

  • bxl/foo.bxl:50, in _impl
    ctx.output.print(" lib outputs: {}".format(get_lib_outputs(ctx, target)))
  • bxl/foo.bxl:14, in get_lib_outputs
    ctx.output.print(ctx.fs.abs_path_unsafe(output))
    error: Type of parameter expr doesn't match, expected artifact | file_node | str, actual artifact
    --> bxl/foo.bxl:14:22
    |
    14 | ctx.output.print(ctx.fs.abs_path_unsafe(output))
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |

which is complaining the expected type (artifact) and actual type (artifact) don't match.

Anything else I can try?

@ndmitchell
Copy link
Contributor

CC @wendy728 (I have no idea)

@wendy728
Copy link
Contributor

Have you tried get_path_without_materialization()? That should work with the artifact type. As for the error message about artifact and artifact not matching, this is an unfortunate typing issue where the starlark types are the same, but the underlying rust types are different. We would need to do some kind of migration to fix that. Regardless, abs_path_unsafe() is not the right function here since that's only for source artifacts, not declared or build/output artifacts.

I will add a some docs about that method since people have a hard time finding it in our generated docs, and also work on improving the error message for abs_path_unsafe().

As for the restrictions on ensured artifact, yes the API is intentionally restrictive so that people don't accidentally use the paths as strings, which might break in actions ran remotely, for example. See more. here.

@zjturner
Copy link
Contributor Author

zjturner commented Dec 21, 2023

I’ll try this. Is there any reason it would be a bad idea for abs_path_unsafe to work with any kind of artifact?

I think what’s difficult about finding this function is that it doesn’t really make sense as a global. There’s already ctx.fs with similar methods. Or why not just as a member function on the artifact itself? These are the natural places one would expect to find such a method

@wendy728
Copy link
Contributor

The functions available on ctx.fs are for source files/artifacts. So abs_path_unsafe works with string literals that represent source files, actual source file nodes, or cell paths that are supposed to be source files, and there are no plans to make it work with build artifacts or other types of artifacts.

@zjturner
Copy link
Contributor Author

zjturner commented Dec 21, 2023

In that case, how about a method on the artifact object? I tried get_path_without_materializations(), and it worked perfectly, im just trying to think from an api design standpoint so that more things become self documenting / self discoverable. It's a very unusual and specific function to be a global.

I actually have a local patch that adds this method to output artifact, im just wondering if it’s worth trying to upstream or if it will be seen as making it too easy to do unsafe things.

I understand the desire to make it difficult to shoot yourself in the foot by doing unsafe things, but at the same time one of the primary motivating use cases of bxl (IDE project generation) has heavy reliance on these kinds of unsafe operations.

@wendy728
Copy link
Contributor

The issue is that the artifact type is shared between normal rules and BXL. We could probably get it to only be exposed in BXL, but I would have to do some investigation on how to implement it correctly, but also that could be confusing if an object method is only available in BXL vs rules.

@zjturner
Copy link
Contributor Author

Just my own 2c, but I think bxl is a natural place for methods which are really unsafe for rules but quite natural when you want to introspect. IDE project generation, for example, often relies on absolute paths, but you’d never want to use an absolute path in a rule. So bxl should make it easy to get absolute paths for all kinds of things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants