Skip to content
This repository has been archived by the owner on Sep 15, 2021. It is now read-only.

Allow stubbing out some transitive dependencies #185

Closed
c-parsons opened this issue Apr 25, 2019 · 11 comments
Closed

Allow stubbing out some transitive dependencies #185

c-parsons opened this issue Apr 25, 2019 · 11 comments
Assignees

Comments

@c-parsons
Copy link
Contributor

It would be nice if Stardoc was able to function without having the full transitive set of bzl files available (as not all of them are relevant in Stardoc's processing; Stardoc only needs approximately "loading phase" evaluation of the Starlark code)

A couple of options:

  1. When missing an import, Stardoc could automatically attempt to fake any import symbols (using a nicemock equivalent)
  2. We could allow a user to specify the labels of bzl files they would prefer to stub out instead of actually loading.

This might make for easier adoption in cases where bzl_libary targets are not created for one's transitive dependencies.

@adam-azarchs
Copy link

A probably particularly-common example of where this would be helpful is for documenting rules that depend on stuff in bazel_tools, for for example @bazel_tools//tools/build_defs/pkg:pkg.bzl, since that does not export a bzl_library, and the only target that seems to make that file visible is @bazel_tools//tools:srcs, which is broken right now (depending on targets which do not exist).

@c-parsons
Copy link
Contributor Author

Indeed (might help fix #166)

@tmc
Copy link
Contributor

tmc commented Jun 1, 2019

@adam-azarchs is there an issue open for the @bazel_tools//tools:srcs issue?

@adam-azarchs
Copy link

Not that I am aware of, unless you count #166.

@c-parsons
Copy link
Contributor Author

I have a proof of concept to solve this with option (1), though I have some concerns with going forward with it.

I'd like to describe the proposal here:

"When missing an import, Stardoc will automatically treat any symbols that would have been defined by setting them to None".

This may work in to resolve any cases where there's a transitive dependency which loads a symbol which isn't used in important ways (for example, if the symbol is used in any rule function -- Stardoc doesn't invoke those!)

However, I have two concerns:

  1. When a missing loaded symbol is used, but the unexpect None type causes an error, it may be difficult to trace the initial error.

For example, suppose we aim to load a missing function my_func:

load("@missing_dep//dep:dep.bzl", "my_func")

x = my_func()

We will get an error that NoneType is not callable, which makes it unclear that this was caused because the "@missing_dep//dep:dep.bzl" dependency could not be resolved.
To alleviate this, we could log every dependency which we "fake" in this way, and, on any error, we can ensure the error message contains information about which dependencies were faked.

...but is that good enough?

  1. When a missing loaded symbol is used but doesn't cause an error, and instead causes documentation to appear unlike it should. For example, suppose the docstring of a rule (or even worse, an attribute name) is processed partially by evaluating a symbol which could not be successfully loaded. For example:
load("@missing_dep//dep:dep.bzl", "my_const")

my_rule = rule(...,
    doc = "Lorem ipsum" + str(my_const))

We could mitigate this by making it sufficiently difficult to interact with a missing symbol in meaningful ways. For example, we could make missing symbols not swapped in with None, and instead use a new Starlark object type which doesn't support repr or str (and throws an exception instead. This does not solve the issue, however, because the significant logic could instead be assessing the type of the object (which is supported by all Starlark types).

Alternative considerations

  • We might consider option (2), where users need to describe the specific dependencies they want faked. However, this still runs into the same issues as above (a faked dependency may be used in strange ways, and it may be unclear that the specific dependency was at fault, even if selected). It may also be a large maintenance burden to continuously add missing dependencies to the stardoc target.
  • We may want this functionality to be attached to a feature flag, which is opt-in with appropriate "experimental" disclaimer. (It will at least not surprise current users).

@c-parsons
Copy link
Contributor Author

@jin @laurentlb for your consideration

@laurentlb
Copy link
Contributor

Is there any way to make @bazel_tools available?

@c-parsons
Copy link
Contributor Author

Is there any way to make @bazel_tools available?

The only reasonable approach I can think of here is to expose (public visibility) a glob of all Starlark files under @bazel_tools, and then have a bzl_library target in bazel_skylib which points to this glob.

@adam-azarchs
Copy link

That target exists, @bazel_tools//tools:srcs, but last I checked is broken due to some missing files.

@c-parsons
Copy link
Contributor Author

Looks like @bazel_tools//tools:srcs has been broken for some time.
There's also @bazel_tools//tools:embedded_tools_srcs, which looks based on it's description like it might fit best here, but that also doesn't build...

Might be worth fixing that separately -- for now, I wonder if introducing a new target to comprise only all Starlark files under @bazel_tools is worth it.

@c-parsons
Copy link
Contributor Author

This is not fixed in an ideal manner, as of yet, but will have a decent workaround with the next bazel release:

Add @bazel_tools//tools:bzl_srcs to the dependencies of your bzl_library target.

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

No branches or pull requests

5 participants