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

Documentation issue: Make Variables #14033

Open
daveb68 opened this issue Sep 24, 2021 · 5 comments
Open

Documentation issue: Make Variables #14033

daveb68 opened this issue Sep 24, 2021 · 5 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)

Comments

@daveb68
Copy link

daveb68 commented Sep 24, 2021

Documentation URL: https://docs.bazel.build/versions/main/be/make-variables.html

The following statement appears to be incorrect:

All referenced labels must appear in the consuming rule's srcs, output files, or deps. Otherwise the build fails. C++ rules can also reference labels in data.

I believe that for adding files to the C++ linking stage, the proper attribute to add a file to use for "Predefined source/output path variables" is additional_linker_inputs.

I couldn't figure out how to add a file to the linking stage by reading the documentation without some guidance from another developer in another organization. According to the documentation (and trials), "srcs" must be c++ source or header files, "deps" must be targets of type cc_library or objc_library, and the documentation for "data" state it is only for runtime files.

It seems that I am failing to grok the bazel essentials from reading the documentation. Can you provide some guidance on what I should be doing to be able to resolve build design issues with bazel correctly and efficiently? Is it my personal failure in understanding the documentation or something else I should be doing, such as reviewing the bazel java code?

The following is what I finally came up with to solve this issue:

cc_binary(
...
    additional_linker_inputs = ["my-tests-prejs.js"],
    linkopts = [
        "--pre-js $(rootpath :my-tests-prejs.js)",  # works
        #"--pre-js //learn/cpp/emscripten:my-tests-prejs.js", # fails
        #"--pre-js my-tests-prejs.js",  # Fails
        #"--pre-js :my-tests-prejs.js",  # Fails
]
...
)

C++ linkopts refers to "https://docs.bazel.build/versions/main/be/common-definitions.html#label-expansion" for label expansion. "Label Expansion" currently state:

Some string attributes of a very few rules are subject to label expansion: if those strings contain a valid label as a substring, such as //mypkg:target, and that label is a declared prerequisite of the current rule, it is expanded into the pathname of the file represented by the target //mypkg:target.

Example attributes include genrule.cmd and cc_binary.linkopts. The details may vary significantly in each case, over such issues as: whether relative labels are expanded; how labels that expand to multiple files are treated, etc. Consult the rule attribute documentation for specifics.

You can see from my my code sample my initial interpretation of this section by all of the '# fails' comments.

I do not know how I would have connected the dots on my own without diving into the java source code or just trying different things until maybe something worked. Any guidance that would permit me to solve future issues on my own would be greatly appreciated.

@oquenchil
Copy link
Contributor

We can improve the documentation for cc_binary to clarify how to pass a path from a file dependency.

As for future issues I cannot really comment without knowing the actual issue. Anywhere where you think that the documentation needs improvement, please let us know.

Thanks.

@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: documentation (cleanup) labels Sep 27, 2021
@daveb68
Copy link
Author

daveb68 commented Sep 27, 2021

As for future issues I cannot really comment without knowing the actual issue. Anywhere where you think that the documentation needs improvement, please let us know.

I am just trying to make sure I have taken all reasonable steps to be proficient with the bazel tools and am able to efficiently solve any issue that one considered competent in the use of bazel to "design and create" build environments would be able to. Not being able to solve that last issue on my own shook my confidence. I am looking for guidance to understand if the shortcoming is my own and what I might do to resolve it. OR, to understand how others solve issues such as this one if there was no shortcoming.

@oquenchil
Copy link
Contributor

Here in particular I would have said that you showed way more proficiency than usual by going into the Bazel code-base itself and figuring it out. If there is a short-coming on our side about something not well documented, there isn't much more you can do.

In this case I don't know what exactly your thoughts were and for how long you stayed at each step. Maybe you thought that the label would be converted automatically to a path and if you had seen the path in a command line you would have noticed earlier that something is wrong. If that's the case, then perhaps the following could have helped:
bazel build --verbose_failures

will show you the Bazel action that failed. In your case you would have seen a cc_binary linking action failing and on the command like you would have seen a *-2.params file. If you open that file you would see the Bazel label rather than the actual location of the file.

Sometimes you want to find out the path to the *-2.params file even if nothing failed. In those cases, you can do bazel build once. Followed by deleting the produced binary. Then run bazel build -s. The option -s will show you the command line for every single action in the build. The reason why I said build once, then delete the file, is so that everything else that you are not interested is pulled from the cache and is not listed with -s.

@daveb68
Copy link
Author

daveb68 commented Sep 27, 2021

Thank you very much for the response. I feel better about my effort.

I had already been using the -s to view sub commands after "bazel clean" to ensure I was seeing all of the commands executed for a "test target".

For this particular case, I think the documentation for cc_binary's 'additional_linker_inputs' including an example, such as below would have been golden.

 additional_linker_inputs = ["my-tests-prejs.js"],
    linkopts = [
        "--pre-js $(rootpath :my-tests-prejs.js)",  # works

@ShreeM01 ShreeM01 added the team-Documentation Documentation improvements that cannot be directly linked to other team labels label Jan 10, 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 Mar 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Documentation Documentation improvements that cannot be directly linked to other team labels team-Rules-CPP Issues for C++ rules type: documentation (cleanup)
Projects
None yet
Development

No branches or pull requests

3 participants