WORKSPACE allows repository redefinition

[hanwen]

this has a test, so this is apparently expected behavior,

bazel/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java

Line 919 in 1b4b341

"local_repository(name = 'foo', path = '/bar')",

but this is really confusing, if we have a repo twice due to some error. At the very least, the error message should point to both definitions of the repository.

$ cat WORKSPACE

Succeeds if commenting out following

load("//:test.bzl", "BLA")

LUCENE_VERS = '5.5.2'

maven_jar(
name = 'lucene_misc',
artifact = 'org.apache.lucene:lucene-misc:' + LUCENE_VERS,
sha1 = '0000000000000000000000000000000000000000',
)

maven_jar(
name = 'lucene_misc',
artifact = 'org.apache.lucene:lucene-misc:' + LUCENE_VERS,
sha1 = '37bbe5a2fb429499dfbe75d750d1778881fff45d',
)

$ bazel build //...
ERROR: /usr/local/google/home/hanwen/vc/bazel-bug/WORKSPACE:12:1: Cannot redefine repository after any load statement in the WORKSPACE file (for repository 'lucene_misc').
ERROR: Error evaluating WORKSPACE file.
ERROR: package contains errors: .
ERROR: error loading package 'external': Package 'external' contains errors.
INFO: Elapsed time: 0.100s

[hanwen]

introduced by 466873e

I summon @damienmg

[damienmg]

Yes this is a feature and we should document it, maybe we should restrict the redinition to only repository defined by bazel too (bazel_tools, android_*, ...)

[damienmg]

Ok after a quick though, we should do:

1. document how the workspace is split/interpreted
2. only allow overwriting of bazel defined repositories to avoid abusing the feature.
3. document that defined repository are overwrittable before the first "split" of the workspace.

[ramtej]

Hello,

I have a similar issue using the "bazel-deps" from https://github.com/johnynek/bazel-deps while
having two sub-workspaces included using the "local_repository" rule.

1) Environment

• Ubuntu 16.04, 4.4.0-81-generic py_binary correctly forwards arguments #104-Ubuntu SMP, x86_64
• $bazel version
  Build label: 0.5.1
  Build target: bazel-out/..
  Build time: Tue Jun 6 10:34:11 2017 (1496745251)
  Build timestamp: 1496745251
  Build timestamp as int: 1496745251

2) Steps

$ git clone https://github.com/ramtej/bazel-multiworkspace-sampler.git
$ tree bazel-multiworkspace-sampler/ -L 3

bazel-multiworkspace-sampler/
├── LICENSE
├── profile # Development "Profile". A composition of all required components for the current tasks.
│ ├── BUILD
│ └── WORKSPACE
├── README.md
└── workspaces
├── a.app # Application A.app (e.g. frontend.app)
│ ├── 3rdparty
│ ├── BUILD
│ ├── src
│ ├── WORKSPACE
│ └── workspace.bzl
└── b.app # Application B.app (e.g. backend.app)
├── 3rdparty
├── BUILD
├── src
├── WORKSPACE
└── workspace.bzl

2.1) To build a single app works well

$ cd bazel-multiworkspace-sampler/workspaces/a.app/
$ bazel build //...

2.2) To build a single app from the "profile" workspace does not work, due to the
"Cannot redefine repository after any load statement in the WORKSPACE file" error message.

$ cd bazel-multiworkspace-sampler/profile/
$ bazel build @a_app//...

b_app/3rdparty/load.bzl", line 5, in callback
native.maven_jar(name = item["name"], artifact = it..."], ...)
Cannot redefine repository after any load statement in the WORKSPACE file (for repository 'asm_asm').

The native.maven_jar rules from the b_app project that resolves the external "asm" dependency
?conflicts? with the already resolved "asm" dependency by project a_app.

---

With a locally patched bazel version (simply removed the allowOverride flag) the above setup works fine. But I'm not sure what this "overriding" means in detail and what are the usecases.. and how to set the flag from e.g. WORKSPACE file.

Thank you.

Best Regards,
J. Jetmar

[damienmg]

@ramtej we should not allow repository redifinition because they can create correctness issues, we allow them at the top of the WORKSPACE file only (before any load statement) to allow redefinition of Bazel implicit workspace (e.g. bazel_tools), we should lock down the allow override to those and only those.

[ramtej]

@damienmg thank you for the feedback. I'm not sure if I got it fully.

I updated the profile/WORKSPACE file as follows:

local_repository(
name = "a_app",
path = "../workspaces/a.app",
)

local_repository(
name = "b_app",
path = "../workspaces/b.app",
)

load("@a_app//:workspace.bzl", "a_app")
a_app()

load("@b_app//:workspace.bzl", "b_app")
b_app()

Note : ../workspaces/a.app/WORKSPACE and ../workspaces/b.app/WORKSPACE contains no load() statement. The profile/WORKSPACE uses the rule "local_repository" to load targets from the sub-workspaces /a.app and /b.app. AFTER the /a.app and /b.app sub-workspaces are loaded (local_repository rule) the load() rule are used to load targets from both a.app/b.app workspace.bzl. But still the "Cannot redefine repository after any load statement in the WORKSPACE file" error message is generated.

The question is now, how to load local repositories? Is there any valid sample? I used the tensorflow approach as a template for my java based project.

Thank you

Best regards,
jj

[damienmg]

If b_app() redefine external repositories defined in a_app() then you have a repository re-definition and the load("@a_app//:workspace.bzl", "a_app") stop allowing repository redefinition.

[damienmg]

PS: I think this is correct but you should simply have a check using native.existing_rule to avoid redefining twice the same repository in a_app/b_app.

[ramtej]

Hello,
thank you - I added the native.existing_rule check to the load.bzl to avoid redefinition. Works now!
This is not yet a perfect solution as two maven artifacts with different versions will have the same
maven_jar rule name, but this is another issue.

Thank you again !

Best Regards,
jj

PS :
Updates are pushed to https://github.com/ramtej/bazel-multiworkspace-sampler