From e11592a06eb0a51ce0fbcf2f90e9339862d29be1 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Sat, 31 Mar 2018 12:28:34 -1000 Subject: [PATCH 1/3] Add a ThriftInfo provider --- thrift/thrift.bzl | 31 +++++++++++++++-------------- twitter_scrooge/twitter_scrooge.bzl | 23 +++++++++++---------- 2 files changed, 28 insertions(+), 26 deletions(-) diff --git a/thrift/thrift.bzl b/thrift/thrift.bzl index 89ec136cf..c4090fc9b 100644 --- a/thrift/thrift.bzl +++ b/thrift/thrift.bzl @@ -2,6 +2,8 @@ _thrift_filetype = FileType([".thrift"]) +ThriftInfo = provider(fields=["srcs", "transitive_srcs", "external_jars", "transitive_external_jars"]) + def _common_prefix(strings): pref = None for s in strings: @@ -87,30 +89,29 @@ rm {out}.contents jarfiles.append(depset(jar.files)) transitive_external_jars = depset(transitive = jarfiles) - return struct( - thrift = struct( - srcs = ctx.outputs.libarchive, - transitive_srcs = transitive_srcs, - external_jars = ctx.attr.external_jars, - transitive_external_jars = transitive_external_jars, - ), - ) + return [ + ThriftInfo( + srcs = ctx.outputs.libarchive, + transitive_srcs = transitive_srcs, + external_jars = ctx.attr.external_jars, + transitive_external_jars = transitive_external_jars, + )] -def _collect_thrift_attr_depsets(targets, attr): +def _collect_thrift_srcs(targets): ds = [] for target in targets: - ds.append(getattr(target.thrift, attr)) + ds.append(target[ThriftInfo].transitive_srcs) return ds -def _collect_thrift_srcs(targets): - return _collect_thrift_attr_depsets(targets, "transitive_srcs") - def _collect_thrift_external_jars(targets): - return _collect_thrift_attr_depsets(targets, "transitive_external_jars") + ds = [] + for target in targets: + ds.append(target[ThriftInfo].transitive_external_jars) + return ds def _valid_thrift_deps(targets): for target in targets: - if not hasattr(target, "thrift"): + if not target[ThriftInfo]: fail("thrift_library can only depend on thrift_library", target) # Some notes on the raison d'etre of thrift_library vs. code gen specific diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index 7e34accce..82c773354 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -5,6 +5,8 @@ load("//scala:scala.bzl", "collect_srcjars", "collect_jars") +load("//thrift:thrift.bzl", "ThriftInfo") + _jar_filetype = FileType([".jar"]) def twitter_scrooge(): @@ -57,8 +59,8 @@ def twitter_scrooge(): def _collect_transitive_srcs(targets): r = [] for target in targets: - if hasattr(target, "thrift"): - r.append(target.thrift.transitive_srcs) + if ThriftInfo in target: + r.append(target[ThriftInfo].transitive_srcs) return depset(transitive = r) def _collect_owned_srcs(targets): @@ -73,11 +75,10 @@ def _collect_owned_srcs(targets): def _collect_external_jars(targets): r = [] for target in targets: - if hasattr(target, "thrift"): - thrift = target.thrift - if hasattr(thrift, "external_jars"): - for jar in thrift.external_jars: - r.append(depset(_jar_filetype.filter(jar.files))) + if ThriftInfo in target: + thrift = target[ThriftInfo] + for jar in thrift.external_jars: + r.append(depset(_jar_filetype.filter(jar.files))) r.append(depset(_jar_filetype.filter(thrift.transitive_external_jars))) return depset(transitive = r) @@ -92,11 +93,11 @@ def collect_extra_srcjars(targets): return depset(srcjar, transitive = srcjars) def _collect_immediate_srcs(targets): - r = [] + srcs = [] for target in targets: - if hasattr(target, "thrift"): - r.append(depset([target.thrift.srcs])) - return depset(transitive = r) + if ThriftInfo in target: + srcs.append(target[ThriftInfo].srcs) + return depset(srcs) def _assert_set_is_subset(want, have): missing = [] From 264408135590f7ae6536d945e5dd23b5244b6608 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Sat, 31 Mar 2018 12:44:21 -1000 Subject: [PATCH 2/3] minor cleanup --- thrift/thrift.bzl | 2 +- twitter_scrooge/twitter_scrooge.bzl | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/thrift/thrift.bzl b/thrift/thrift.bzl index c4090fc9b..50e1c9b22 100644 --- a/thrift/thrift.bzl +++ b/thrift/thrift.bzl @@ -111,7 +111,7 @@ def _collect_thrift_external_jars(targets): def _valid_thrift_deps(targets): for target in targets: - if not target[ThriftInfo]: + if not ThriftInfo in target: fail("thrift_library can only depend on thrift_library", target) # Some notes on the raison d'etre of thrift_library vs. code gen specific diff --git a/twitter_scrooge/twitter_scrooge.bzl b/twitter_scrooge/twitter_scrooge.bzl index 82c773354..2cc0f8a90 100644 --- a/twitter_scrooge/twitter_scrooge.bzl +++ b/twitter_scrooge/twitter_scrooge.bzl @@ -78,9 +78,9 @@ def _collect_external_jars(targets): if ThriftInfo in target: thrift = target[ThriftInfo] for jar in thrift.external_jars: - r.append(depset(_jar_filetype.filter(jar.files))) - r.append(depset(_jar_filetype.filter(thrift.transitive_external_jars))) - return depset(transitive = r) + r.extend(_jar_filetype.filter(jar.files)) + r.extend(_jar_filetype.filter(thrift.transitive_external_jars)) + return depset(r) def collect_extra_srcjars(targets): srcjar = [] From ba16278c67dcce3db95b53b0e5c2ddc26a1136e5 Mon Sep 17 00:00:00 2001 From: "P. Oscar Boykin" Date: Sat, 31 Mar 2018 12:47:49 -1000 Subject: [PATCH 3/3] address review comments --- thrift/thrift.bzl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/thrift/thrift.bzl b/thrift/thrift.bzl index 50e1c9b22..8a8de26d3 100644 --- a/thrift/thrift.bzl +++ b/thrift/thrift.bzl @@ -2,7 +2,13 @@ _thrift_filetype = FileType([".thrift"]) -ThriftInfo = provider(fields=["srcs", "transitive_srcs", "external_jars", "transitive_external_jars"]) +ThriftInfo = provider( + fields=[ + "srcs", # The source files in this rule + "transitive_srcs", # the transitive version of the above + "external_jars", # external jars of thrift files + "transitive_external_jars" # transitive version of the above + ]) def _common_prefix(strings): pref = None