thought-machine / please

High-performance extensible build system for reproducible multi-language builds.
https://please.build
Apache License 2.0
2.44k stars 203 forks source link

Strange plz build issue when dependency is incorrectly referenced with double `//` #55

Closed fsiddiqia closed 8 years ago

fsiddiqia commented 8 years ago

You can find the relevant test case in core3/ branch model-python-server-test

We have a BUILD file like this (noe the double slash in the last dependency). (core3/experimental/diana/models/frontend/src/js/components/monitor/BUILD)

js_library(
  name = 'monitor',
  srcs = ['monitor.js'],
  deps = [
    '//experimental/diana/models/frontend/src/js/components/header:header',
    '//experimental/diana/models/frontend/src/js/services:monitor_service',
    '//experimental/diana/models/frontend/src/js/services:benchmark_service',
    '//experimental/diana/models/frontend/src/js/components/graphs:size_graph',
    '//experimental/diana/models/frontend/src/js/components/graphs:bench_graph',
    '//experimental/diana/models/frontend/src/js/components//graphs:time_graph',
    '//third_party/js:react',
  ],
  visibility = ['//experimental/diana/models/...'],
)

Building that project results in the error below. Running a second time it is successful!

~/dev/core3  model-python-server-test ✔                                                                                                                                   45m  ⍉
▶ plz build //experimental/diana/models/frontend/src/...                             
Building [78/166, 0.2s]:
=> [ 0.0s] //third_party/js:babel-preset-react_npm Finished post-build function for //third_party/js:babel-preset-react_npm
=> [ 0.0s] //third_party/js:babel-plugin-syntax-trailing-function-commas_npm Preparing...
=> [ 0.0s] //third_party/js:webpack-dev-server_npm Preparing...
=> [ 0.0s] //third_party/js:css-loader_npm Running post-build function for //third_party/js:css-loader_npm
=> [ 0.0s] //third_party/js:chokidar_npm Finished post-build function for //third_party/js:chokidar_npm
=> [-0.0s] //third_party/js:babel-relay-plugin_npm Finished post-build function for //third_party/js:babel-relay-plugin_npm
=> [-0.0s] //third_party/js:imports-loader_npm Running post-build function for //third_party/js:imports-loader_npm
=> [-0.0s] //third_party/js:chalk_npm Running post-build function for //third_party/js:chalk_npm
=> [-0.0s] //third_party/js:fs-extra_npm Running post-build function for //third_party/js:fs-extra_npm
=> [-0.0s] //third_party/js:extract-text-webpack-plugin_npm Running post-build function for //third_party/js:extract-text-webpack-plugin_npm
10:24:46.702 CRITICAL: //experimental/diana/models/frontend/src/js/components/graphs:time_graph failed:
Error moving outputs for target //experimental/diana/models/frontend/src/js/components/graphs:time_graph: Rule //experimental/diana/models/frontend/src/js/components/graphs:time_graph failed to create output plz-out/tmp/experimental/diana/models/frontend/src/js/components/graphs/time_graph/timeGraph.js

~/dev/core3  model-python-server-test ✔                                                                                                                                   46m  ⍉
▶ plz build //experimental/diana/models/frontend/src/...
Build finished; total time 13.56s, incrementality 92.6%. Outputs:
  plz-out/gen/experimental/diana/models/frontend/src/css/main.scss
  plz-out/gen/experimental/diana/models/frontend/src/js/compiled.js
  plz-out/gen/experimental/diana/models/frontend/src/js/compiled.js.css
  plz-out/gen/experimental/diana/models/frontend/src/js/compiled.js.css.map
  plz-out/gen/experimental/diana/models/frontend/src/js/compiled.js.map
  plz-out/gen/experimental/diana/models/frontend/src/js/components/graphs/benchGraph.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/graphs/sizeGraph.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/graphs/timeGraph.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/header/header.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/header/headerData.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelBenchmark.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelName.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelSize.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelTime.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelUncompressedSize.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/model/modelVersion.js
  plz-out/gen/experimental/diana/models/frontend/src/js/components/monitor/monitor.js
  plz-out/gen/experimental/diana/models/frontend/src/js/services/BenchmarkService.js
  plz-out/gen/experimental/diana/models/frontend/src/js/services/MonitorService.js

I think the problem is that it fails the first time (but manages to place it in the cache) and passes the second time because it's going through a different code path since the artifact is in the cache.

peterebden commented 8 years ago

I think what's going on here is that we actually get two different packages (one with // and one without) which both correspond to the same file because various calls normalise out the //, and it then races in some places when creating / deleting files; it certainly seems very nondeterministic to me. As you say, the cache seems to play a part; --nocache seems to affect results too.

Should be relatively easily fixed by banning the // anyway...