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.

Sign up for GitHub

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

Jump to bottom

python-binary-create task maps all product directories to the same target #4389

Closed
derrley opened this issue Mar 27, 2017 · 0 comments · Fixed by #4390
Closed

python-binary-create task maps all product directories to the same target #4389

derrley opened this issue Mar 27, 2017 · 0 comments · Fixed by #4390

Comments

@derrley
Copy link
Contributor

derrley commented Mar 27, 2017

An example, from my project, wherein two targets, docker/monitoring_sidecar:monitoring_sidecar and tools/templating:configure are supposed to produce two pex files, monitoring_sidecar.pex and configure.pex (accordingly). Instead, the product mapping has both of them mapped to the first target, instead of each mapped to its own target.

ProductMapping(pex_archives) {
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/docker.monitoring_sidecar.monitoring_sidecar/current
    [u'monitoring_sidecar.pex']
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/tools.templating.configure/current
    [u'configure.pex']
}

This breaks a pants plugin I've written that assembles docker images by depending on python_binary and jvm_binary targets. It creates docker's workspace dynamically based on its dependencies' product mappings.
derrley pushed a commit to derrley/pants that referenced this issue Mar 27, 2017
Fix pantsbuild#4389: python-binary-create task maps all product direc…
50e7fda 
…tories to the same target
@derrley derrley mentioned this issue Mar 27, 2017
Merged
derrley pushed a commit to derrley/pants that referenced this issue Mar 27, 2017
Fix pantsbuild#4389: python-binary-create task maps all product direc…
6aa5614 
…tories to the same target
kwlzn pushed a commit that referenced this issue Mar 27, 2017
@derrley @kwlzn
python-binary-create task maps all product directories to the same ta…
afc17b4 
…rget (#4390)

Problem

This fixes #4389

Solution

I've simply noticed that the binary variable was erroneously used outside the scope of its for loop, and replaced that use with vt.targets, which was likely what the original author intended. Using the wrong variable, in this case, caused all pex mappings to be mapped from the same target (whatever wound up being the terminal for loop case).

Result

When this patch is applied, product mappings are assembled correctly. From the example in #4389 , here's what the product mapping looks like now:

ProductMapping(pex_archives) {
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/docker.monitoring_sidecar.monitoring_sidecar/current
    [u'monitoring_sidecar.pex']
  PythonBinary(BuildFileAddress(BuildFile(tools/templating/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), configure)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/tools.templating.configure/current
    [u'configure.pex']
}
derrley added a commit to derrley/pants that referenced this issue Mar 27, 2017
@derrley
python-binary-create task maps all product directories to the same ta…
1a09b8c 
…rget (pantsbuild#4390)

Problem

This fixes pantsbuild#4389

Solution

I've simply noticed that the binary variable was erroneously used outside the scope of its for loop, and replaced that use with vt.targets, which was likely what the original author intended. Using the wrong variable, in this case, caused all pex mappings to be mapped from the same target (whatever wound up being the terminal for loop case).

Result

When this patch is applied, product mappings are assembled correctly. From the example in pantsbuild#4389 , here's what the product mapping looks like now:

ProductMapping(pex_archives) {
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/docker.monitoring_sidecar.monitoring_sidecar/current
    [u'monitoring_sidecar.pex']
  PythonBinary(BuildFileAddress(BuildFile(tools/templating/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), configure)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/tools.templating.configure/current
    [u'configure.pex']
}
peiyuwang pushed a commit to peiyuwang/pants that referenced this issue Mar 29, 2017
@derrley @peiyuwang
python-binary-create task maps all product directories to the same ta…
063fac6 
…rget (pantsbuild#4390)

Problem

This fixes pantsbuild#4389

Solution

I've simply noticed that the binary variable was erroneously used outside the scope of its for loop, and replaced that use with vt.targets, which was likely what the original author intended. Using the wrong variable, in this case, caused all pex mappings to be mapped from the same target (whatever wound up being the terminal for loop case).

Result

When this patch is applied, product mappings are assembled correctly. From the example in pantsbuild#4389 , here's what the product mapping looks like now:

ProductMapping(pex_archives) {
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/docker.monitoring_sidecar.monitoring_sidecar/current
    [u'monitoring_sidecar.pex']
  PythonBinary(BuildFileAddress(BuildFile(tools/templating/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), configure)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/tools.templating.configure/current
    [u'configure.pex']
}
@jsirois jsirois mentioned this issue Apr 21, 2017
Closed
lenucksi pushed a commit to lenucksi/pants that referenced this issue Apr 25, 2017
@derrley @lenucksi
python-binary-create task maps all product directories to the same ta…
0b86ee4 
…rget (pantsbuild#4390)

Problem

This fixes pantsbuild#4389

Solution

I've simply noticed that the binary variable was erroneously used outside the scope of its for loop, and replaced that use with vt.targets, which was likely what the original author intended. Using the wrong variable, in this case, caused all pex mappings to be mapped from the same target (whatever wound up being the terminal for loop case).

Result

When this patch is applied, product mappings are assembled correctly. From the example in pantsbuild#4389 , here's what the product mapping looks like now:

ProductMapping(pex_archives) {
  PythonBinary(BuildFileAddress(BuildFile(docker/monitoring_sidecar/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), monitoring_sidecar)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/docker.monitoring_sidecar.monitoring_sidecar/current
    [u'monitoring_sidecar.pex']
  PythonBinary(BuildFileAddress(BuildFile(tools/templating/BUILD, FileSystemProjectTree(/Users/kyle.derr/Repositories/allclearid/odin/mono)), configure)) => /Users/kyle.derr/Repositories/allclearid/odin/mono/.pants.d/binary/python-binary-create/0550b754d96b/tools.templating.configure/current
    [u'configure.pex']
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #4389: python-binary-create task maps all product directories to the same target derrley/pants
1 participant
@derrley