Skip to content

Commit fff5a37

Browse files
Catherine Gasniermeta-codesync[bot]
authored andcommitted
Fix hsthrift build (internal + OSS)
Summary: ## Summary This diff fixes two related build issues for hsthrift: 1. **Internal build failure**: The Glean Haskell indexer was failing with `Unstructured annotations are not allowed: 'haxl.batched'` 2. **OSS build failure**: The GitHub CI build was/will be failing because thrift annotation files couldn't be found ### Background The `math.thrift` file previously used an unstructured annotation: ```thrift oneway void putMany(1: list<i64> val) (haxl.batched); ``` This was fixed by a codemod in **October 2025** (D83550305) which converted it to a structured annotation. However, that fix was **reverted in November 2025** (D86966535) by Simon Marlow because it broke the GitHub CI - the `thrift/annotation/thrift.thrift` file wasn't accessible in the OSS build environment. The revert commit message stated: > *"This broke github CI. The file `thrift/annotations/thrift.thrift` is not present in the hsthrift github repo, and this thrift file is just a test."* Since then, enforcement of the unstructured annotation ban has become stricter in fbsource, causing the internal Glean Haskell indexer to fail (see Sandcastle workflow [508906757907574458](https://www.internalfb.com/sandcastle/workflow/508906757907574458)). Additionally, in early **March 2026**, codemods ran across fbsource adding `include "thrift/annotation/thrift.thrift"` to other files in hsthrift that are compiled during the OSS build: - `tests/if/hs_test.thrift` (March 2, 2026) - `tests/if/foo.thrift` (March 2, 2026) - `tests/if/constants.thrift` - `tests/if/map.thrift` This means the OSS build is now broken regardless of whether `math.thrift` is fixed - other files also need the annotation include path. ### The Fix **1. `lib/test/if/math.thrift`** - Convert unstructured to structured annotation: ```thrift include "thrift/annotation/thrift.thrift" ... thrift.DeprecatedUnvalidatedAnnotations{items = {"haxl.batched": "1"}} oneway void putMany(1: list<i64> val); ``` **2. `Makefile`** - Add include path for fbthrift annotation files: ```makefile HSTHRIFT_PREFIX ?= $(HOME)/.hsthrift THRIFT_INCLUDE := -I $(HSTHRIFT_PREFIX)/include ``` Then use `$(THRIFT_INCLUDE)` in the `thrift-cpp` target when invoking `thrift1`. When fbthrift is installed via `new_install_deps.sh` to `~/.hsthrift/`, the thrift compiler can now find the annotation files at `~/.hsthrift/include/thrift/annotation/thrift.thrift`. ### Why this fixes both issues - **Internal build**: Uses the structured annotation syntax that the Thrift compiler now requires - **OSS build**: The Makefile now passes `-I ~/.hsthrift/include` to `thrift1`, allowing it to find `thrift/annotation/thrift.thrift` which is installed as part of fbthrift Reviewed By: malanka, aahanaggarwal Differential Revision: D96733254 fbshipit-source-id: 13c480f9a4cd11fc8ce4c2211fd47808e6046a99
1 parent 49fb109 commit fff5a37

2 files changed

Lines changed: 11 additions & 4 deletions

File tree

Makefile

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ CABAL_BIN := cabal
77
THRIFT1 := thrift1
88
CABAL := $(CABAL_BIN) $(CABAL_CONFIG_FLAGS) $(GETDEPS_CABAL_FLAGS)
99

10+
# Include path for fbthrift annotation files (installed by new_install_deps.sh)
11+
HSTHRIFT_PREFIX ?= $(HOME)/.hsthrift
12+
THRIFT_INCLUDE := -I $(HSTHRIFT_PREFIX)/include
13+
1014
# Targets in this file invoke Cabal and hence can't be built in parallel
1115
.NOTPARALLEL:
1216

@@ -122,13 +126,13 @@ thrift-hs:: compiler
122126

123127
thrift-cpp::
124128
mkdir -p cpp-channel/if cpp-channel/test/if
125-
cd lib && $(THRIFT1) -I . --gen mstch_cpp2 \
129+
cd lib && $(THRIFT1) $(THRIFT_INCLUDE) -I . --gen mstch_cpp2 \
126130
-o ../cpp-channel/if \
127131
if/RpcOptions.thrift
128-
cd lib/test/if && $(THRIFT1) -I . --gen mstch_cpp2 \
132+
cd lib/test/if && $(THRIFT1) $(THRIFT_INCLUDE) -I . --gen mstch_cpp2 \
129133
-o ../../../cpp-channel/test/if \
130134
math.thrift
131-
cd tests/if && $(THRIFT1) -I . --gen mstch_cpp2 \
135+
cd tests/if && $(THRIFT1) $(THRIFT_INCLUDE) -I . --gen mstch_cpp2 \
132136
-o . \
133137
hs_test.thrift
134138

lib/test/if/math.thrift

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
* LICENSE file in the root directory of this source tree.
77
*/
88

9+
include "thrift/annotation/thrift.thrift"
10+
911
package "facebook.com/hs/thrift/test"
1012

1113
namespace cpp2 "cpp2"
@@ -30,7 +32,8 @@ service Calculator extends Adder {
3032

3133
oneway void put(1: i64 val);
3234

33-
oneway void putMany(1: list<i64> val) (haxl.batched);
35+
@thrift.DeprecatedUnvalidatedAnnotations{items = {"haxl.batched": "1"}}
36+
oneway void putMany(1: list<i64> val);
3437

3538
i64 get();
3639

0 commit comments

Comments
 (0)