Copilot commented on code in PR #3944:
URL: https://github.com/apache/thrift/pull/3944#discussion_r4157584332
##########
lib/go/test/Makefile.am:
##########
@@ -73,10 +73,10 @@ gopath: $(THRIFT) $(THRIFTTEST) \
ForwardType.thrift \
StringParseAllocationTest.thrift \
DocCommentTest.thrift
- mkdir -p gopath/src
- grep -v 'set<Insanity>' $(THRIFTTEST) > ThriftTest.thrift
+ mkdir -p gopath/src build
+ grep -v 'set<Insanity>' $(THRIFTTEST) > build/ThriftTest.thrift
$(THRIFT) $(THRIFTARGS) $(RECURSIVE)
- $(THRIFT) $(THRIFTARGS) -r IncludesTest.thrift
+ $(THRIFT) $(THRIFTARGS) -I build -r IncludesTest.thrift
Review Comment:
The compiler resolves an include relative to the including IDL before
consulting `-I` paths (`compiler/cpp/src/thrift/main.cc:361-381`). An existing
ignored `lib/go/test/ThriftTest.thrift` left by the previous rule will
therefore shadow the newly generated `build/ThriftTest.thrift`, so an upgraded
working tree can silently generate from stale input. Remove the legacy artifact
before invoking the compiler (and preferably continue removing it from
`clean-local`).
##########
test/go/Makefile.am:
##########
@@ -48,7 +45,7 @@ bin/stress: gopath
$(GO) build $(GOBUILDEXTRA) -o bin/stress ./src/bin/stress
clean-local:
- $(RM) -r src/gen src/github.com/golang src/thrift bin pkg gopath
ThriftTest.thrift
+ $(RM) -r src/gen src/github.com/golang src/thrift bin pkg gopath
Review Comment:
Keep deleting the legacy generated `ThriftTest.thrift` here. After updating
an existing checkout, the previously ignored copy remains untracked, but this
PR both exposes it in `git status` and removes the only cleanup path, so `make
clean` can no longer restore a clean source tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
