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

Data dependencies make reference to workspace #22162

Open
lgalfaso opened this issue Apr 27, 2024 · 7 comments
Open

Data dependencies make reference to workspace #22162

lgalfaso opened this issue Apr 27, 2024 · 7 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: documentation (cleanup)

Comments

@lgalfaso
Copy link

Page link:

https://bazel.build/concepts/dependencies#data-dependencies

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

The page in the link reads

These files are available using the relative path path/to/data/file. In tests, you can refer to these files by joining the paths of the test's source directory and the workspace-relative path, for example, ${TEST_SRCDIR}/workspace/path/to/data/file

The reference to workspace in this link would make you believe that you can make a reference to any workspace, including the workspace for any dependency.

Eg. Say you have a dependency foo. This dependency has a BUILD file at the root that reads

filegroup(
    name = "bar",
    srcs = glob(["man/**/*.txt"]),
)

Say that @foo//:bar contains 1000s of files.

Then you have a cc_test that you would like to use the files from @foo//:bar.

cc_test(
    name = "some_test",
    srcs = ["some_test.cpp"],
    data = ["@foo//:bar"],
)

then following the doc, I would expect for ${TEST_SRCDIR}/foo/man to be reference to the man directory in @foo//:bar (for completeness, say that the workspace name of @foo is "foo").

This is not the case (and my understanding is that this is not the expected behaviour).

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?

No response

@lgalfaso lgalfaso added team-Documentation Documentation improvements that cannot be directly linked to other team labels type: documentation (cleanup) untriaged labels Apr 27, 2024
@sgowroji sgowroji added the team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. label Apr 29, 2024
@meteorcloudy meteorcloudy added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels May 28, 2024
@Wyverald
Copy link
Member

Which Bazel version are you using? I tested on Bazel 7.1.2 and I could see that ${TEST_SRCDIR}/foo/man does indeed contain the files I expected.

@lgalfaso
Copy link
Author

@Wyverald the path depends on whether you are importing the dependency from WORKSPACE or from MODULE.bazel. If you are using the former, then you are correct that it will be available at ${TEST_SRCDIR}/foo/man. If you are using the later following the patter from #17141 (comment), then it will be available at ${TEST_SRCDIR}/_main~_foo/man.

This is using Basel 7.1.2.

@Wyverald
Copy link
Member

That is true. @fmeum do you know if the runfiles library works with ${TEST_SRCDIR}? And/or what's the best practice here? We should fix the docs.

@fmeum
Copy link
Collaborator

fmeum commented May 30, 2024

Yes, runfiles libraries generally also support tests via TEST_SRCDIR. If you want consistent behavior between WORKSPACE and Bzlmod, try using @bazel_tools//tools/cpp/runfiles.

+1 to fixing up the docs.

@lgalfaso
Copy link
Author

@fmeum is there a way for @bazel_tools//tools/cpp/runfiles to make a reference to a directory?

@fmeum
Copy link
Collaborator

fmeum commented May 30, 2024

You can reference a file in it and then walk up from there. Directory lookups could be added, but that would require a filesystem abstraction that I'm not sure is feasible to implement on C++ (std::filesystem may support this, but I don't know it well enough).

@lgalfaso
Copy link
Author

Having to reference a file, the fact that at the moment there is a (maybe bad) alternative for which this is not needed, makes the option of just using ${TEST_SRCDIR}/_main~_foo/man much better. This shows the abstraction that Bazel adds, but the alternative feels much hackier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

7 participants