Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

genrule(): Make deps to targets used in $(exe //path/to:target) and $(location //path/to:target) macros implicitly #128

Closed
davido opened this issue Jun 1, 2014 · 3 comments

Comments

@davido
Copy link
Contributor

davido commented Jun 1, 2014

Buck handles dependencies accross different rules inconistently. For all these rules the dependency to :foo will be made implicitly:

java_library(
  name = 'lib',
  srcs = [:foo],
)

prebuilt_jar(
  name = 'jar',
  binary_jar = ':foo',
)

java_binary(
  name = 'app',
  manifest_file = ':foo',
)

In this light it's kind of unexpected, that macros $(exe //path/to:target) and $(location //path/to:target) require me to list the dependencies explicitly, as it's stated in the documentation:

[...]
Note that all build rules expanded in the command must be listed
in the deps parameter of the genrule().

So while this currently has to be:

genrule(
  name = 'foo',
  cmd = '$(exe :bar) $(location :baz)',
  deps = [
    ':bar',
    ':baz',
  ],
)

this obviously must just be (especially following DRY principle):

genrule(
  name = 'foo',
  cmd = '$(exe :bar) $(location :baz)',
)
@davido davido changed the title genrule(): Make deps to $(exe :foo) and $(location :bar) implicitly genrule(): Make deps to targets used in $(exe //path/to:target) and $(location //path/to:target) macros implicitly Jun 1, 2014
@sdwilsh sdwilsh added bug and removed bug labels Jun 1, 2014
@sdwilsh
Copy link
Contributor

sdwilsh commented Jun 1, 2014

We should totally fix this. Good catch!

@natthu natthu closed this as completed in d632c70 Jun 9, 2014
@davido
Copy link
Contributor Author

davido commented Jun 10, 2014

@sdwilsh @natthu Thanks for the quick fix. We have applied it on our tree: [1].

[1] https://gerrit-review.googlesource.com/#/c/57713

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jun 11, 2014
Number of bugs were fixed in recent Buck version:

* java_binary: manifest_file rejects target name as input [1]
* java_binary: optionally suppress manifest merging from dependent
JARs [2]
* java_library(): changing deps to provided_deps doesn't trigger
library rebuild [3]
* genrule: Make deps to $(exe :foo) and $(location :bar) implicitly
[4]

[1] facebook/buck#126
[2] facebook/buck#87
[3] facebook/buck#133
[4] facebook/buck#128

Change-Id: I8fb9f57676eac242b54892a50d90773bfc5cb0c0
openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jun 11, 2014
Apply [1] on our Buck tree.

[1] facebook/buck#128

Change-Id: Ie3a9de0b2ee3725170bcbd06df059ea5d9ae5252
@spearce
Copy link
Contributor

spearce commented Jan 5, 2015

The documentation still says the deps is required:

http://facebook.github.io/buck/rule/genrule.html

$(location //path/to:target)
Expands to the location of the output of the build rule. This means that you can refer to these without needing to be aware of how Buck is storing data on the disk mid-build.
Note that all build rules expanded in the command must be listed in the deps parameter of the genrule().

openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jan 7, 2015
Since [1] Buck implicitly adds :foo and :bar in $(exe :foo) and
$(location :bar) macros to the dependency list of genrule(). The
code base was adjusted to reflect this enhancement in Ie3a9de0b2.
In I996b8d2a1 the unneeded dependencies were partially re-added
in CM integration code.

Re-remove it again.

[1] facebook/buck#128
Change-Id: I8d0aaa5c9f7586f04f169f8362709b1fc7b04dcc
estebank pushed a commit to estebank/gitiles that referenced this issue Jan 30, 2015
Apply [1] on our Buck tree.

[1] facebook/buck#128

Change-Id: Ia00a9fa2e4786a961e84fd5266ef97f364cec61e
uwolfer pushed a commit to gerrit-review/gerrit that referenced this issue Jun 8, 2015
Since [1] dependencies to targets used in $(exe //path/to:target) and
$(location //path/to:target) macros are added implicitly.

[1] facebook/buck#128
Change-Id: Id98257e1118830205821e35816d0e562e56e961a
openstack-gerrit pushed a commit to openstack-infra/gerrit that referenced this issue Jun 18, 2015
Since [1] dependencies to targets used in $(location //path/to:target)
macro are added implicitly.

This can be verified by using Bucks audit and targets commands:

  $ buck targets --json api
  [...]
  "buck.type" : "genrule",
  "name" : "api",
  "deps" : [ ],
  [...]

So explicit dependencies are empty, but implicit dependencies are still
present:

  $ buck audit dependencies api
  //gerrit-extension-api:extension-api
  //gerrit-extension-api:extension-api-javadoc
  //gerrit-extension-api:extension-api-src
  //gerrit-plugin-api:plugin-api
  //gerrit-plugin-api:plugin-api-javadoc
  //gerrit-plugin-api:plugin-api-src
  //gerrit-plugin-gwtui:gwtui-api
  //gerrit-plugin-gwtui:gwtui-api-javadoc
  //gerrit-plugin-gwtui:gwtui-api-src

[1] facebook/buck#128

Change-Id: I6c8ddb40ef95ee45f6b6605e74381d55524229af
dpursehouse added a commit to gerrit-review/gerrit that referenced this issue Oct 7, 2015
Since [1] dependencies to targets used in $(location //path/to:target)
macro are added implicitly.

Similar build rules simplification was applied in I6c8ddb40ef and in
Id98257e111.

[1] facebook/buck#128

Change-Id: Ife9717f37a9cdf55358da61b7e9df26fdf23c501
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants