"foo-bar is not a legal workspace name" error is reported on Bazel@HEAD

[davido]

It seems that on Bazel@HEAD (e6cce76) hyphen character is not accepted any more in workspace name?

/** A collection of global Starlark build API functions that apply to WORKSPACE files. */
public class WorkspaceGlobals implements WorkspaceGlobalsApi {

  // Must start with a letter and can contain letters, numbers, and underscores
  private static final Pattern LEGAL_WORKSPACE_NAME = Pattern.compile("^\\p{Alpha}\\w*$");

Am I missing something? I see these errors:

 $ bazel build org.eclipse.jgit.ssh.apache/...
[...]
ERROR: /home/davido/.cache/bazel/_bazel_davido/e1ee1c2b4d81e57e5c4b6fbb219eece8/external/sshd-osgi/WORKSPACE:2:10: sshd-osgi is not a legal workspace name
ERROR: /home/davido/.cache/bazel/_bazel_davido/e1ee1c2b4d81e57e5c4b6fbb219eece8/external/sshd-sftp/WORKSPACE:2:10: sshd-sftp is not a legal workspace name
ERROR: /home/davido/.cache/bazel/_bazel_davido/e1ee1c2b4d81e57e5c4b6fbb219eece8/external/log-api/WORKSPACE:2:10: log-api is not a legal workspace name
INFO: Analyzed target //org.eclipse.jgit.ssh.apache:ssh-apache (28 packages loaded, 1339 targets configured).
INFO: Found 1 target...

The error is reported for the workspace line, e.g. in external/log-api/WORKSPACE:2:10:

# DO NOT EDIT: automatically generated WORKSPACE file for maven_jar rule
workspace(name = "log-api")

The build still succeeds.

[thii]

It looks like the hyphen character has never been accepted in a workspace name. But for some reason, Bazel didn't error out in some cases; for instance, new workspaces declared with new_git_repository.

[davido]

It looks like the hyphen character has never been accepted in a workspace name.

Gerrit Code Review and JGit projects are using hyphen character in workspace names for years, started from Bazel 0.6.

See this CL: [1] from July 2018, quoting the commit message:

Bazel: Harmonize names of external repositories

Recent Bazel versions support dash character in external repository
names. Consistently use them with one exception: javax_inject. This is
needed to match the name in the rules_closure.

[1] https://gerrit-review.googlesource.com/c/gerrit/+/183530

[laurentlb]

Do you have a repro for this? I'd like to understand what changed exactly.

I had the same error with Bazel 3.4.1 (and previous versions):

$ cat WORKSPACE
workspace(name = "log-api")
$ bazel --version
bazel 3.4.1
$ bazel build :all
ERROR: /tmp/bzl/WORKSPACE:1:10: log-api is not a legal workspace name

[laurentlb]

Repro:

$ cat WORKSPACE
local_repository(
    name = "busybox",
    path = "sub",
)

$ cat sub/WORKSPACE 
workspace(name = "busybox_1.25.0")

$ USE_BAZEL_VERSION=3.4.1 bazelisk build @busybox//...
INFO: Analyzed 0 targets (0 packages loaded, 0 targets configured).
INFO: Found 0 targets...
INFO: Elapsed time: 0.300s, Critical Path: 0.01s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

$ USE_BAZEL_VERSION=3.5.0rc2 bazelisk build @busybox//...
Starting local Bazel server and connecting to it...
ERROR: Traceback (most recent call last):
	File "/usr/local/google/home/laurentlb/.cache/bazel/_bazel_laurentlb/1cf9e271f53a4fcee3da31675bf7c3a0/external/busybox/WORKSPACE", line 1, column 10, in <toplevel>
		workspace(name = "busybox_1.25.0")
Error in workspace: busybox_1.25.0 is not a legal workspace name
INFO: Analyzed 0 targets (1 packages loaded, 0 targets configured).
INFO: Found 0 targets...
INFO: Elapsed time: 2.521s, Critical Path: 0.03s
INFO: 0 processes.
INFO: Build completed successfully, 1 total action

[davido]

Do you have a repro for this?

Try to build gerrit project from HEAD (https://gerrit.googlesource.com/gerrit/):

  $ bazel build :gerrit

With 3.4.1 it worked, no errors reported even with "foo-bar" external workspace names.
In 3.5.1rc1 it is reporting errors. I haven't bisected that regression though.

[laurentlb]

Root cause: 2436a35

cc @michajlo

[michajlo]

#11837 (comment) is correct, it appears allowing hyphens was a misbug.

[aiuto]

@davido: This looks like log spam rather than a build failure.

There seem to be entirely different paths forward

1. Revert the change to allow hyphens, people ignore the error messages for now. (with variations on maybe adding an ability to silence the message).
2. Change the api to allow hyphens and dots.

We will need @alandonovan to weigh in on the choice.

[davido]

There seem to be entirely different paths forward

I see another option:

Revert the root cause for this regression: 2436a35 and accept dot and hyphens in point release 3.5.

Announce that starting from Bazel 4.0 dot and hyphens are not supported any more in workspace names and roll forward: 2436a35. Moreover change the API and make it no only log spam but a real build breakage.

[ulfjack]

TensorFlow also uses hyphens in workspace names, at least in some cases:

ERROR: Traceback (most recent call last):
	File "/home/ulfjack/.cache/bazel/_bazel_ulfjack/e1ebd4c707b223a2af1441f90d605454/external/ubuntu16.04-manylinux2010-py3_config_platform/WORKSPACE", line 2, column 10, in <toplevel>
		workspace(name = "ubuntu16.04-manylinux2010-py3_config_platform")
Error in workspace: ubuntu16.04-manylinux2010-py3_config_platform is not a legal workspace name
ERROR: Traceback (most recent call last):
	File "/home/ulfjack/.cache/bazel/_bazel_ulfjack/e1ebd4c707b223a2af1441f90d605454/external/ubuntu16.04-manylinux2010-py3_config_python/WORKSPACE", line 2, column 10, in <toplevel>
		workspace(name = "ubuntu16.04-manylinux2010-py3_config_python")
Error in workspace: ubuntu16.04-manylinux2010-py3_config_python is not a legal workspace name

[davido]

TensorFlow also uses hyphens in workspace names, at least in some cases:

Note, that in Bazel 3.5.0rc2 the hyphen char is accepted in workspace name. In the error message you pasted: ubuntu16.04-manylinux2010-py3_config_python the problematic char is dot and not the hyphen.

[alandonovan]

We should allow hyphens; this is now done. I would go a step further and say we should allow any printable character except the three required for label parsing: @, :, and /.

(I regard Blaze's totalitarian inspection of every string it accepts as a youthful mistake.)

[michajlo]

... we should allow any printable character except the three required for label parsing: @, :, and /.

Let's keep it to just the things we need for now... As we've observed here it's a lot easier to allow a new char than realize we made a mistake and try to take it back.

[davido]

Let's keep it to just the things we need for now...

That would mean that we should allow dot character and we are done, right (hyphen was already added in my recent PR)?

[michajlo]

Correct.

[aiuto]

I'm counting this as concensus.

• I will shortly send a PR to allow dot in the name.
• When it lands I will cherrypick into the next Bazel 3.5 rc.

[laurentlb]

I'm not sure this cherrypick is necessary. The root cause is that Bazel will evaluate and report errors from workspace files when it shouldn't.

The use of dot and dashes is just one example from the bug report above. That doesn't mean anything regarding what symbols the other projects are using.

[aiuto]

@davido This should be resolved in 3.5.0rc3 in 2 ways.

• Hyphens and dots are both allowed
• We no longer parse external WORKSPACE files just to find their name (and then ignore it).

I've done my own local tests on the candidate. Please LMK if you still see this issue with that.

[davido]

@aiuto

Thanks. Will test 3.5.0rc3 later today and let you know the result.

[aiuto]

You're the best.

…

On Tue, Aug 18, 2020 at 10:19 AM David Ostrovsky ***@***.***> wrote: @aiuto <https://github.com/aiuto> Thanks. Will test 3.5.0rc3 later today and let you know the result. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11837 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHBOYEFDTJL556NSOULSBKEWZANCNFSM4PHO5XFQ> .

[davido]

Will test 3.5.0rc3 later today and let you know the result.

Confirmed, that 3.5.0rc3 works as expected. Thanks again for your help.

  $ bazel version
Build label: 3.5.0rc3

  $ bazel build :release
Starting local Bazel server and connecting to it...
INFO: Invocation ID: 3778213c-4029-486a-b8f3-ec8185b3bfa4
INFO: Analyzed target //:release (1297 packages loaded, 42305 targets configured).
INFO: Found 1 target...
INFO: From Bundling JavaScript plugins/delete-project/delete-project-bundle.js [rollup]:

plugins/delete-project/gr-delete-repo/plugin.js → bazel-out/k8-fastbuild/bin/plugins/delete-project/delete-project-bundle.js...
created bazel-out/k8-fastbuild/bin/plugins/delete-project/delete-project-bundle.js in 161ms
INFO: From Bundling JavaScript polygerrit-ui/app/gr-app-bundle-js.js [rollup]:
bazel-out/k8-fastbuild/bin/polygerrit-ui/app/_pg_ts_out/elements/gr-app.js → bazel-out/k8-fastbuild/bin/polygerrit-ui/app/gr-app-bundle-js.js...
created bazel-out/k8-fastbuild/bin/polygerrit-ui/app/gr-app-bundle-js.js in 25.6s
Target //:release up-to-date:
  bazel-bin/release.war
INFO: Elapsed time: 203.822s, Critical Path: 140.59s
INFO: 762 processes: 638 remote cache hit, 65 linux-sandbox, 1 local, 58 worker.
INFO: Build completed successfully, 864 total actions