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

Crash when macro param default value is a select() #40

Closed
lisaonduty opened this issue Nov 15, 2019 · 16 comments
Closed

Crash when macro param default value is a select() #40

lisaonduty opened this issue Nov 15, 2019 · 16 comments

Comments

@lisaonduty
Copy link

lisaonduty commented Nov 15, 2019

Description of the problem:

I tried to use Stardoc to generate som documents and then I ran into a problem when select() was used. I got the following error:

Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = 'cannot use %r substitution placeholder when simplifiedFormatStrings is set'

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

To show some more simple example I just made a very small update into the Bazel C++ tutorial and I got the same error.
I added .bazelrc in stage2 directory and a new file hello-world.bzl in the main sub-directory and updated the BUILD file in the same sub-directory.

.../examples/cpp-tutorial/stage2 -> 
README.md
WORKSPACE
lib
main
.bazelrc

.../examples/cpp-tutorial/stage2/main -> 
BUILD
hello-greet.cc
hello-greet.h
hello-time.cc
hello-time.h
hello-world.bzl

In the .bazelrc file I added the following:

build:apple  --fruit=apple
build:orange  --fruit=orange
build:banana  --fruit=banana

And the hello-world.bzl have the following content:

DEFAULT_FRUIT_CONFIG = (select({
    "apple": "red",
    "banana": "yellow",
    "orange": "orange",
}))

def fruit_colour_with_select(fruit_colour = DEFAULT_FRUIT_CONFIG):
    """
    **fruit_colour_with_select** finds out colour of the fruit.
    """
    return(fruit_colour)

def fruit_colour_no_select(fruit_colour = "blue"):
    """
    **fruit_colour_no_select** returns fruit colour blue.
    """
    return(fruit_colour)

I added the following into the BUILD file:

load(
    "@io_bazel_stardoc//stardoc:stardoc.bzl",
    "stardoc"
)
load(
    "@bazel_skylib//:bzl_library.bzl",
    "bzl_library"
)

config_setting(
    name = "apple",
    values = {"fruit": "apple"},
)

config_setting(
    name = "banana",
    values = {"fruit": "banana"},
)

config_setting(
    name = "orange",
    values = {"fruit": "orange"},
)

stardoc(
    name = "hello-world-docs-no-select",
    input = "hello-world.bzl",
    out = "hello_world_doc_no_select.md",
    symbol_names = [
        "fruit_colour_no_select",]
)
stardoc(
    name = "hello-world-docs-with-select",
    input = "hello-world.bzl",
    out = "hello_world_doc_with_select.md",
    symbol_names = [
        "fruit_colour_with_select",]
)

It works to generate document for target hello-world-docs-no-select but not for hello-world-docs-with-select:

OK:
bazel build //main:hello-world-docs-no-select 
NOK:
bazel build //main:hello-world-docs-with-select 
bazel build //main:hello-world-docs-with-select  --define="fruit=apple"
.../examples/cpp-tutorial/stage2 -> bazel build //main:hello-world-docs-no-select 
INFO: Invocation ID: ba902970-4211-47e8-87cd-c5939095e0f9
INFO: Analyzed target //main:hello-world-docs-no-select (0 packages loaded, 1 target configured).
INFO: Found 1 target...
Target //main:hello-world-docs-no-select up-to-date:
  bazel-bin/main/hello_world_doc_no_select.md
INFO: Elapsed time: 0.131s, Critical Path: 0.01s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

.../examples/cpp-tutorial/stage2 -> bazel build //main:hello-world-docs-with-select 
INFO: Invocation ID: e369f3cb-685b-4954-b6d6-a84ba5e59404
INFO: Analyzed target //main:hello-world-docs-with-select (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: /repo/.../examples/cpp-tutorial/stage2/main/BUILD:47:1: Generating proto for Starlark doc for hello-world-docs-with-select failed (Exit 1) stardoc failed: error executing command 
  (cd /repo/$USER/bazel_build_root/testbaselib/sandbox/processwrapper-sandbox/6/execroot/__main__ && \
  exec env - \
  bazel-out/host/bin/external/io_bazel_stardoc/stardoc/stardoc '--input=//main:hello-world.bzl' '--workspace_name=' '--symbols=fruit_colour_with_select' '--dep_roots=.' '--dep_roots=external/' '--output=bazel-out/k8-fastbuild/bin/main/hello-world-docs-with-select.raw')
Execution platform: @bazel_tools//platforms:host_platform

Use --sandbox_debug to see verbose messages from the sandbox
Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = 'cannot use %r substitution placeholder when simplifiedFormatStrings is set'
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.formatWithList(Printer.java:495)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.format(Printer.java:453)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.format(Printer.java:188)
	at com.google.devtools.build.lib.syntax.SelectorValue.repr(SelectorValue.java:81)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.repr(Printer.java:279)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.appendListElements(Printer.java:405)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.printList(Printer.java:387)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.printList(Printer.java:188)
	at com.google.devtools.build.lib.syntax.SelectorList.repr(SelectorList.java:172)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.repr(Printer.java:279)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.forParam(FunctionUtil.java:89)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.parameterInfos(FunctionUtil.java:138)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.fromNameAndFunction(FunctionUtil.java:72)
	at com.google.devtools.build.skydoc.rendering.ProtoRenderer.appendStarlarkFunctionInfos(ProtoRenderer.java:58)
	at com.google.devtools.build.skydoc.SkydocMain.main(SkydocMain.java:254)
Target //main:hello-world-docs-with-select failed to build
INFO: Elapsed time: 0.688s, Critical Path: 0.59s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

.../examples/cpp-tutorial/stage2 -> bazel build //main:hello-world-docs-with-select  --define="fruit=apple"
INFO: Invocation ID: aee445cf-6787-4f08-9b6c-6a8a28cea73b
INFO: Build option --define has changed, discarding analysis cache.
INFO: Analyzed target //main:hello-world-docs-with-select (0 packages loaded, 259 targets configured).
INFO: Found 1 target...
ERROR: /repo/.../examples/cpp-tutorial/stage2/main/BUILD:47:1: Generating proto for Starlark doc for hello-world-docs-with-select failed (Exit 1) stardoc failed: error executing command 
  (cd /repo/uabelma/bazel_build_root/testbaselib/sandbox/processwrapper-sandbox/7/execroot/__main__ && \
  exec env - \
  bazel-out/host/bin/external/io_bazel_stardoc/stardoc/stardoc '--input=//main:hello-world.bzl' '--workspace_name=' '--symbols=fruit_colour_with_select' '--dep_roots=.' '--dep_roots=external/' '--output=bazel-out/k8-fastbuild/bin/main/hello-world-docs-with-select.raw')
Execution platform: @bazel_tools//platforms:host_platform

Use --sandbox_debug to see verbose messages from the sandbox
Exception in thread "main" java.util.UnknownFormatConversionException: Conversion = 'cannot use %r substitution placeholder when simplifiedFormatStrings is set'
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.formatWithList(Printer.java:495)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.format(Printer.java:453)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.format(Printer.java:188)
	at com.google.devtools.build.lib.syntax.SelectorValue.repr(SelectorValue.java:81)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.repr(Printer.java:279)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.appendListElements(Printer.java:405)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.printList(Printer.java:387)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.printList(Printer.java:188)
	at com.google.devtools.build.lib.syntax.SelectorList.repr(SelectorList.java:172)
	at com.google.devtools.build.lib.syntax.Printer$BasePrinter.repr(Printer.java:279)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.forParam(FunctionUtil.java:89)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.parameterInfos(FunctionUtil.java:138)
	at com.google.devtools.build.skydoc.rendering.FunctionUtil.fromNameAndFunction(FunctionUtil.java:72)
	at com.google.devtools.build.skydoc.rendering.ProtoRenderer.appendStarlarkFunctionInfos(ProtoRenderer.java:58)
	at com.google.devtools.build.skydoc.SkydocMain.main(SkydocMain.java:254)
Target //main:hello-world-docs-with-select failed to build
INFO: Elapsed time: 0.815s, Critical Path: 0.59s
INFO: 0 processes.
FAILED: Build did NOT complete successfully
seroiuvd05211:.../examples/cpp-tutorial/stage2 -> 

What operating system are you running Bazel on?

Linux

What's the output of bazel info release?

0.28.1
Now we have 3.3.1 and it's still the same problem

Have you found anything relevant by searching the web?

No

@lisaonduty
Copy link
Author

We have been able to generate documentation for 27 own rules and macros by using Stardoc. But we have one macro that isn't possible to build with Stardo() due to this select() problem.
We have other coming rules and macros in the pipe which will be needed documentation but I don't know if they have any select() so we will see.
The preferred solution would be that Stardoc is able to resolve the select() when passing in build flags like --define="fruit=apple" according to the above example or if using user defined build settings or platform (or whatever is selected on) use corresponding flag syntax. Then the documentation would have the values that are selected.
Would this be possible?

@c-parsons
Copy link
Collaborator

I'm glad to hear you're having success with several of your rules/macros with Stardoc!

Unfortunately, having Stardoc recognize select() is quite an undertaking, and we don't have the cycles to look into a solution in the near term. It will likely be much easier after the work described in #69 (comment) is complete.

@lisaonduty
Copy link
Author

lisaonduty commented Nov 18, 2020

Thanks @c-parsons for your answer it's much appreciated!

It is really interesting to read about the plans. It sounds like a great idea to make the implementation cleaner and to re-use all functionality that actually already exist in Bazel.
Then I assume the discussions we had about dependencies would be resolved and Stardoc can be used the same way as other Bazel rules.

It would also be much easier to get a grip of how Stardoc works. I think that would be a great benefit and make it easier to contribute. We have other thoughts and wishes that haven't felt enough important to issue tickets on.
For example I see needs for supporting bulleted or numbered lists format inside the Description cell in the table (would need update of util.markdownCellFormat() function in Bazel which formats the cell so it's not possible to just update the template).
Another thing that we have seen need of is to support top alignments of the Name column in the table. When the Description cell contain much text it doesn't work to have the Name aligned in the middle. We have been able to get top alignment by adding html tags in our local template. It's not that easy to read as the Markdown format and not a beauty solution but it works for now.

I am really looking forward to the re-structuring of how Stardoc works and also the other parts I read about via your first link e.g. the Documentation inheritance.

@c-parsons
Copy link
Collaborator

For example I see needs for supporting bulleted or numbered lists format inside the Description cell in the table (would need update of util.markdownCellFormat() function in Bazel which formats the cell so it's not possible to just update the template).

I don't think there are yet any plans to make this easier, and I was unaware of this issue. Does this not work to use markdown format for these purposes? (Can you provide an example and file that separately?)

All-in-all, the overhaul plan I linked to will make it easier to retrieve implementation-related information (from macros & rules), but will not improve string formatting of your doc strings. (That will need to be done separately).

@lisaonduty
Copy link
Author

Yes I can write about these things separately in own tickets. Will do that in short. We haven't thought that they were as important as this one and we haven't perceived it as stardoc() has been so prioritized so we didn't want to add more than necessary. That might have been wrong conclusions of mine.

@brandjon brandjon changed the title Stardoc can't handle select() Crash when macro param default value is a select() Jan 12, 2021
@brandjon
Copy link
Member

Note that the plan is to have documentation information emitted by Bazel's loading phase, e.g. from bazel query or a genrule()-style rule. Stardoc would simply consume and render this information.

This means that select() information would still remain unresolved from the POV of Stardoc. Any notion of supplying a flag value to further evaluate the select() would be an ad hoc addition by Stardoc, not a faithful reflection of the data model in Bazel's loading phase. In other words, it's an extra maintenance burden, and I'm not sure the benefit is worth it.

In any case, Stardoc should not crash regardless, so let's use this bug to track fixing that.

@lisaonduty
Copy link
Author

Hi @brandjon,
Thanks for the response. What I'm after is to have it more like a cquery which can handle variants but is that not a possible way forward for Stardoc?

@ecngtng
Copy link

ecngtng commented Feb 11, 2022

For this bug, we're also trying to study stardoc code to check if we could do some improvements by ourselves.
Before that, I have some questions. Not sure if someone could help to answer?

1st:
I’m a little curious about stardoc file name.
According to the introduction in the link as below.
(https://github.com/bazelbuild/stardoc/blob/master/docs/maintainers_guide.md)
Stardoc's source code currently lives in the Bazel source tree at https://github.com/bazelbuild/bazel/tree/master/src/main/java/com/google/devtools/build/skydoc

But it seems that some files still use Skydocxxx.java in the directory.
I just want to make sure if these files are used by stardoc.

2nd:
To get more quickly familiar with the code, are there any documents/link to introduce stardoc’s code structure?

Thanks a lot!

@ecngtng
Copy link

ecngtng commented Feb 28, 2022

One more question, where is stardoc code? 😅
In maintainer guide and ticket 69 , it said the code location is in https://github.com/bazelbuild/bazel/tree/master/src/main/java/com/google/devtools/build/skydoc.
But in other doc current stardoc release, it said bazelbuild/stardoc is the repo location.

@tetromino
Copy link
Collaborator

tetromino commented Feb 28, 2022

@ecngtng, Stardoc consists of the documentation generator, templates which configure the exact markdown output, and associated Starlark rules that allow Stardoc to be used by Bazel.

The documentation generator source code lives in the Bazel source tree at https://github.com/bazelbuild/bazel/tree/master/src/main/java/com/google/devtools/build/skydoc. The Stardoc repo bundles two jars (stardoc/stardoc_binary.jar and stardoc/renderer_binary.jar) which are built from Bazel source at a specific commit using the update-release-binary.sh script.

The templates and the rules are in the Stardoc repo.

The convoluted dependency between Bazel and Stardoc, and the bundling of pre-built jars, is not ideal. Our plan is to in the future have Bazel emit a low-level representation of the docstrings and other documentation sources of a .bzl file in protobuf format, and then have Stardoc (via a new small binary whose source will live in the Stardoc repo) turn the protos into Markdown.

@ecngtng
Copy link

ecngtng commented Mar 1, 2022

@tetromino Get it. Thanks for your detailed explanation!

@dgoldstein0
Copy link

I just encountered this bug with my usage of stardoc. Are there any workarounds? I'd be fine if the docs generated don't represent the select correctly, what's not fine is to crash all of doc generation - the latter makes stardoc unusable for me, and I really don't want to tailor my usage of bazel to what stardoc can handle.

IMO this bug is way worse than a p3 from my perspective - until there's a workaround I'm probably going to stop using stardoc entirely.

@dgoldstein0
Copy link

... and of course right after I post this I realize I can just use None as my default and hide the select() in the macro, as a workaround

@tetromino
Copy link
Collaborator

As far as I can tell, this crash has been fixed - I cannot reproduce it with Stardoc 0.6.2 + Bazel 7.0.1 or with Stardoc 0.6.2 + Bazel 6.4.0

@dgoldstein0 - what versions of Stardoc and Bazel you were using when you saw the crash? If it's the latest ones, would you be able to provide a way to reproduce the problem?

@dgoldstein0
Copy link

it's been a while, but we're still on bazel 6.1.0 (so I believe it was at most that) as well as stardoc 0.4.0. I looked into upgrading stardoc but we haven't used bazel modules for anything yet and the newer version had a bunch more deps that made the non-module setup rather hard to figure out so I gave up after ~30m of trying. I'd definitely like to be on newer versions of both.

So yeah, older versions of stuff. Understandable if that means we have to suffer the old bugs 😢

@tetromino
Copy link
Collaborator

Closing as obsolete (since the issue has been fixed in recent releases).

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

No branches or pull requests

6 participants