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

Error messaging when BUILD file not present at top level directory is unclear #9364

Closed
nlopezgi opened this issue Sep 10, 2019 · 11 comments
Closed
Labels
bad error messaging Issues where users get stuck because they don't understand what they did wrong more data needed P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged

Comments

@nlopezgi
Copy link
Contributor

Description of the problem / feature request:

go_repository now reads a configuration file, which, by default is WORKSPACE in the main workspace.
The implementation of this feature is such that the error messaging when the file is not present is as follows:

ERROR: Analysis of target '//src/hack/project:image' failed; build aborted: no such package '@com_github_google_go_containerregistry//pkg/v1': Unable to load package for //:WORKSPACE: BUILD file not found in any of the following directories.
 - /local/path/to/my/git/repository

This error message is already causing issues for rules_docker customers, as now this BUILD file is needed in all projects that depend transitively on gazelle.

Please see bazel-contrib/bazel-gazelle#609 (comment)

Also related:
bazelbuild/rules_docker#1139
bazelbuild/rules_docker#1126

Feature requests: what underlying problem are you trying to solve with this feature?

I would like to have better messaging for this type of errors.

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

Depend on gazelle (latest version) from a project that does not have a root BUILD file

fyi @laurentlb

@irengrig irengrig added P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged labels Sep 11, 2019
@laurentlb laurentlb added the bad error messaging Issues where users get stuck because they don't understand what they did wrong label Sep 11, 2019
@laurentlb laurentlb assigned aehlig and unassigned laurentlb Sep 11, 2019
@meisterT
Copy link
Member

What message would you suggest?

@aehlig
Copy link
Contributor

aehlig commented Sep 12, 2019

Just to clarify @meisterT's question.

The quoted issue

Please see bazelbuild/bazel-gazelle#609 (comment)

states

A better error message would be awesome as it did take a couple of minutes for my head to jolt and remember the error message corresponded to a missing build file.

However, the error message literally states BUILD file not found, so I wonder what additional information you would like to have seen.

@nlopezgi
Copy link
Contributor Author

BUILD file not found at the top level directory
would be a good start

Note error message currently includes:
Analysis of target '//src/hack/project:image' failed
and
failed; build aborted: no such package '@com_github_google_go_containerregistry//pkg/v1'
and
Unable to load package for //:WORKSPACE:

So user has to work out that the two first references above are misleading, and the latter one
Unable to load package for //:WORKSPACE: refers to not having a BUILD file at that level.

As evidenced by 2 issues opened by different users in rules_docker, I'm not the only one who feels this message is not informative enough.
bazelbuild/rules_docker#1139
bazelbuild/rules_docker#1126

Other error traces from those issues:

ERROR: Analysis of target '//dockertest:dockertest' failed; build aborted: no such package '@com_github_pkg_errors//': Unable to load package for //:WORKSPACE: BUILD file not found in any of the following directories.
 - /Users/jeffhodges/src/github.com/jmhodges/bazel_bugs

and

ERROR: @io_bazel_rules_docker//container/go/cmd/digester:go_default_library depends on @com_github_pkg_errors//:go_default_library in repository @com_github_pkg_errors which failed to fetch. no such package '@com_github_pkg_errors//': Unable to load package for //:WORKSPACE: BUILD file not found in any of the following directories.
 - /.../rules_docker/rules_docker_fork/testing/java_image_no_root

I'm sure many would be able to figure out instantly that this means that the BUILD file is missing at the top level, but several others wont.

@aehlig
Copy link
Contributor

aehlig commented Sep 13, 2019

BUILD file not found at the top level directory
would be a good start

Such a message without context would give the wrong impression that there were a requirement to have a BUILD file next next to the WORKSPACE file, which, however, is not a requirement. It is perfectly fine to not have a package for the empty string, as long as no one references the empty string as a package name. In that respect, "top level" is no different than any other package name: only those packages referenced somewhere have to exist.

Therefore, I also wouldn't call ...

So user has to work out that the two first references above are misleading,

... the first two references misleading; instead they give the needed context on where the offending reference was found (keep in mind that bazel has no means of knowing whether your source layout does or does not intend to have a package with name ''). It simply follows the standard pattern of all bazel error messages that a lot of users find useful.

  • First it states which of targets bazel was asked to build failed; this information is not redundant, as the command line accepts target patters that can, in general, expand to several targets.
  • Then it mentions the root cause, in this case a package depended upon that does not exist.
  • Finally, it gives information on the root cause. In this case, why that package does not exist: it has a definition, but that definition depends on an invalid label //:WORKSPACE.
  • It also explains why the label //:WORKSPACE is invalid: that label requires the package '' to exist, but it found no BUILD file for that package.
  • Finally, bazel even lists the candidate directories where a BUILD file could be located in order for the package '' to exist.

This whole chain of reasons is useful to understand why bazel even started to look for that package (in this case with the empty string as name).

So I still don't see any actionable suggestion on how to improve this already very detailed error message.

@nlopezgi
Copy link
Contributor Author

Agreed, thanks anyway

@jin
Copy link
Member

jin commented Sep 13, 2019

Something that'll help here is an additional educational message along the lines of

"Unable to load package for //:WORKSPACE. A BUILD file is required to mark a directory as a Bazel package. BUILD file not found in any of the following directories.."

Many users understand what packages and targets are, but have no idea BUILD files are use to mark package boundaries. I certainly took a while to learn where to place : in target strings.. related #7834

Made a PR: #9385

@nlopezgi wdyt?

@nlopezgi
Copy link
Contributor Author

Hi @jin,
that does look to me like it would be a bit easier to understand,
Thanks for sending that PR

@gnarled-cipher
Copy link
Contributor

gnarled-cipher commented Sep 16, 2019

Just to clarify @meisterT's question.

The quoted issue

Please see bazelbuild/bazel-gazelle#609 (comment)

states

A better error message would be awesome as it did take a couple of minutes for my head to jolt and remember the error message corresponded to a missing build file.

However, the error message literally states BUILD file not found, so I wonder what additional information you would like to have seen.

@aehlig error messages, such as "BUILD file not found...", lack a "call to action". People who are new to Bazel will need to spend more effort to get past the error. This happened to me too (I am new to Bazel)!

In contrast, this message is friendly and makes the most of the end-user's time:

Add a BUILD file to DIRECTORY, to mark it as a Bazel package.

I think that actionable error messages help everyone benefit from using Bazel.

@jin
Copy link
Member

jin commented Sep 16, 2019

@nicoaragon I have an incoming change incorporating your suggestion. Thanks.

@jin jin reopened this Sep 16, 2019
@aehlig aehlig removed their assignment Sep 16, 2019
@aehlig
Copy link
Contributor

aehlig commented Sep 16, 2019

In contrast, this message is friendly and makes the most of the end-user's time:

Add a BUILD file to DIRECTORY, to mark it as a Bazel package.

It, however, it not in all cases the right thing to do. Adding a BUILD file splits the package into which the BUILD file is added, thus rendering other labels incorrect that used to be correct before the addition of the BUILD file.

@gnarled-cipher
Copy link
Contributor

@aehlig

I think you care for accuracy. Also, you want to prevent people breaking their build.

I care for end user experience and I want to empower Bazel users to be comfortable changing, and evolving their build. That includes breaking and fixing the build.

In this case, I would be happy to make the change and break other labels, as long as Bazel lets me know that I did.

What do you use to measure end user experience?

@philwo philwo added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Jun 15, 2020
@philwo philwo removed the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label Nov 29, 2021
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
…iles

    Let's print an actionable for the user and inform that BUILD files turn directories into packages.

    Fixes bazelbuild/bazel#9364

    Closes #9385.

    Change-Id: Ia19133d2bfb0e71fdd03043592f525afddff740d
    PiperOrigin-RevId: 269350796
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bad error messaging Issues where users get stuck because they don't understand what they did wrong more data needed P1 I'll work on this now. (Assignee required) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. untriaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants