pantsbuild / pants / 8605
0%
main: 93%

LAST BUILD BRANCH: tdyas/pytest-9
DEFAULT BRANCH: main
Ran
Jobs 8
Files 0
Run time –
Badge
Embed ▾
README BADGES
x

If you need to use a raster PNG badge, change the '.svg' to '.png' in the link

Markdown

Textile

RDoc

HTML

Rst
8605

push

travis-ci

peiyuwang 
Bug fix: use target.id as bundle prefix to avoid conflict from basenames

We use basename to prefix bundle folder, it has always been unsafe but since we
used to bundle from `target_roots` and people normally bundle one or just a few
targets, the issue isn't obvious.

Since https://rbcommons.com/s/twitter/r/3119/ switching from `target_roots` to
`targets()`, this becomes more problematic, because now jvm_binary that jvm_app
depends on will also be run, if jvm_binary is run after jvm_app, it will destroy
the additional files jvm_app includes. And turns out sharing the same basenames
is pretty common, greping twitter repo 15% basenames have more than 1 occurrence.

See "Testing Done" section for an example from pants own testprojects.

This review

- Switches to use `target.id` as the default bundle prefix, with target.id it's
  safe to bundle transitively from `context.targets()`. Long term this ensures
  correctness.
- basename is still supported to for backward compatibility, but in order to
  reduce the chance people running into this problem, it can only bundle targets
  from `context.target_roots`, essentially goes back to before rb 3119.
- In basename mode, add a detection for duplicate basenames in case that does
  happen so user at least can correct.

NOTE:
- binary has the same problem, I left as a TODO, but let me know if you think we
  should fix it as well, say if junit_tests target wants to depend on binary, this
  will be an issue.
- This review is from Karin's original reviewboard + fixing tests, thanks Karin!
  https://rbcommons.com/s/twitter/r/3250/

Testing Done:
* https://travis-ci.org/pantsbuild/pants/builds/97797692

* Running `./pants bundle testprojects/src/java/org/pantsbuild/testproject/bundle`,
```
On master, note `data/exampledata.txt` is missing

tw-mbp-peiyu:pants peiyu$ find dist/bundle-example-bundle
dist/bundle-example-bundle/bundle-example.jar
dist/bundle-example-bundle/libs/com.google.guava-guava-18.0.jar
...

With the fix

tw-mbp-peiyu:pants peiyu$ find dist/testprojects.src.java.org.pantsbuild.testproject.bundle.bundle-bundle
dist/testprojects.src.java.org.pantsbuild.testproject.bundle.bundle-bundle/bundle-example.jar
dist/testprojects.src.java.org.pantsbuild.testproject.bundle.bundle-bundle/data/exampledata.txt
dist/testprojects.src.java.org.pantsbuild.testproject.bundle.bundle-bundle/libs/com.google.guava-guava-18.0.jar
...
```

Bugs closed: 2745

Reviewed at https://rbcommons.com/s/twitter/r/3272/
Jobs
ID Job ID Ran Files Coverage
1 8605.1 (CI_FLAGS="-cjlpn 'Various pants self checks'") 0
Travis Job 8605.1
4 8605.4 (CI_FLAGS="-fkmsrcjlp 'Python contrib tests'") 0
Travis Job 8605.4
5 8605.5 (CI_FLAGS="-fkmsrjlpn -i 0/6 'Python integration tests for pants - shard 1'") 0
Travis Job 8605.5
6 8605.6 (CI_FLAGS="-fkmsrjlpn -i 1/6 'Python integration tests for pants - shard 2'") 0
Travis Job 8605.6
7 8605.7 (CI_FLAGS="-fkmsrjlpn -i 2/6 'Python integration tests for pants - shard 3'") 0
Travis Job 8605.7
8 8605.8 (CI_FLAGS="-fkmsrjlpn -i 3/6 'Python integration tests for pants - shard 4'") 0
Travis Job 8605.8
9 8605.9 (CI_FLAGS="-fkmsrjlpn -i 4/6 'Python integration tests for pants - shard 5'") 0
Travis Job 8605.9
10 8605.10 (CI_FLAGS="-fkmsrjlpn -i 5/6 'Python integration tests for pants - shard 6'") 0
Travis Job 8605.10
Source Files on build 8605
Detailed source file information is not available for this build.