Skip to content

Commit 97053af

Browse files
authored
parser: refactor startup logic inside of persistent work mode boundary (#161)
* parser: refactor startup logic inside of persistent work mode boundary * fix: auto-detect repo name separator * fix: remove errant filegroup * ignore .claude dir
1 parent 16bff80 commit 97053af

6 files changed

Lines changed: 152 additions & 21 deletions

File tree

BUILD.bazel

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@ load("@build_stack_rules_proto//rules:proto_compile_assets.bzl", "proto_compile_
33
load("@build_stack_scala_gazelle//rules:package_filegroup.bzl", "package_filegroup")
44

55
# -- Gazelle language "walk" ---
6+
# gazelle:exclude .bcr
7+
# gazelle:exclude .claude
68
# gazelle:exclude .github
79
# gazelle:exclude .vscode
8-
# gazelle:exclude .bcr
910
# gazelle:exclude bazel-bin
1011
# gazelle:exclude bazel-out
1112
# gazelle:exclude bazel-scala-gazelle
1213
# gazelle:exclude bazel-testlogs
1314
# gazelle:exclude bin
14-
# gazelle:exclude vendor
1515
# gazelle:exclude examples
16+
# gazelle:exclude vendor
1617

1718
# -- Gazelle language "resolve" ---
1819
# gazelle:resolve go go github.com/stackb/rules_proto/v4/pkg/protoc @build_stack_rules_proto//pkg/protoc

cmd/scalafileextract/scalafileextract.go

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ func run(args []string) error {
5656
cfg.Parser = parser.NewScalametaParser(
5757
parser.WithHttpClientTimeout(60 * time.Second),
5858
)
59-
60-
if err := cfg.Parser.Start(); err != nil {
61-
return fmt.Errorf("starting parser: %w", err)
62-
}
6359
defer func() {
6460
cfg.Parser.Stop()
6561
}()
@@ -72,10 +68,16 @@ func run(args []string) error {
7268
}
7369

7470
if cfg.PersistentWorker {
71+
// In persistent worker mode, parser initialization is deferred
72+
// to inside the loop so startup failures are reported per-request
73+
// via WorkResponse rather than killing the worker process.
7574
if err := persistentWork(&cfg); err != nil {
7675
return fmt.Errorf("while performing persistent work: %v", err)
7776
}
7877
} else {
78+
if err := cfg.Parser.Start(); err != nil {
79+
return fmt.Errorf("starting parser: %w", err)
80+
}
7981
if err := batchWork(&cfg); err != nil {
8082
return fmt.Errorf("while performing batch work: %v", err)
8183
}
@@ -103,16 +105,33 @@ func persistentWork(cfg *Config) error {
103105

104106
batchCfg, err := parseFlags(req.Arguments)
105107
if err != nil {
106-
return fmt.Errorf("parsing work request arguments: %v", err)
107-
}
108+
resp.ExitCode = 1
109+
resp.Output = fmt.Sprintf("parsing work request arguments: %v", err)
110+
} else {
111+
// Ensure parser is running (lazy init + restart if crashed)
112+
if !cfg.Parser.IsRunning() {
113+
cfg.Parser.Stop()
114+
if err := cfg.Parser.Start(); err != nil {
115+
log.Printf("failed to start parser: %v", err)
116+
resp.ExitCode = 1
117+
resp.Output = fmt.Sprintf("starting parser: %v", err)
118+
if err := protobuf.WriteDelimitedTo(&resp, stdout); err != nil {
119+
return fmt.Errorf("writing work response: %v", err)
120+
}
121+
if err := stdout.Flush(); err != nil {
122+
return fmt.Errorf("flushing work response: %v", err)
123+
}
124+
continue
125+
}
126+
}
108127

109-
batchCfg.Parser = cfg.Parser
110-
batchCfg.Cwd = cfg.Cwd
128+
batchCfg.Parser = cfg.Parser
129+
batchCfg.Cwd = cfg.Cwd
111130

112-
if err := batchWork(&batchCfg); err != nil {
113-
// Don't terminate the worker on batch errors; report via WorkResponse
114-
resp.ExitCode = 1
115-
resp.Output = fmt.Sprintf("performing persistent batch: %v", err)
131+
if err := batchWork(&batchCfg); err != nil {
132+
resp.ExitCode = 1
133+
resp.Output = fmt.Sprintf("performing persistent batch: %v", err)
134+
}
116135
}
117136

118137
if err := protobuf.WriteDelimitedTo(&resp, stdout); err != nil {

pkg/maven/BUILD.bazel

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
load("@build_stack_scala_gazelle//rules:package_filegroup.bzl", "package_filegroup")
2-
load("@io_bazel_rules_go//go:def.bzl", "go_library")
2+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
33

44
go_library(
55
name = "maven",
@@ -27,6 +27,18 @@ package_filegroup(
2727
"coordinate.go",
2828
"multiset.go",
2929
"resolver.go",
30+
"resolver_test.go",
3031
],
3132
visibility = ["//visibility:public"],
3233
)
34+
35+
go_test(
36+
name = "maven_test",
37+
srcs = ["resolver_test.go"],
38+
embed = [":maven"],
39+
deps = [
40+
"//pkg/resolver",
41+
"@bazel_gazelle//label",
42+
"@com_github_google_go_cmp//cmp",
43+
],
44+
)

pkg/parser/scalameta_parser.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,17 @@ func (s *ScalametaParser) Stop() {
104104
os.RemoveAll(s.processDir)
105105
s.processDir = ""
106106
}
107+
s.httpPort = 0
108+
s.httpUrl = ""
109+
}
110+
111+
// IsRunning reports whether the parser process is alive.
112+
// ProcessState is set by Wait(); if non-nil the process has exited.
113+
func (s *ScalametaParser) IsRunning() bool {
114+
if s.cmd == nil {
115+
return false
116+
}
117+
return s.cmd.ProcessState == nil
107118
}
108119

109120
func (s *ScalametaParser) Start() error {

pkg/parser/scalameta_parser_test.go

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,81 @@ func mustParseURL(t *testing.T, raw string) *url.URL {
745745
return u
746746
}
747747

748+
func TestIsRunning_NeverStarted(t *testing.T) {
749+
p := NewScalametaParser()
750+
if p.IsRunning() {
751+
t.Error("expected IsRunning() == false for a never-started parser")
752+
}
753+
}
754+
755+
func TestIsRunning_AfterStartAndStop(t *testing.T) {
756+
p := NewScalametaParser()
757+
if err := p.Start(); err != nil {
758+
t.Fatal("Start:", err)
759+
}
760+
if !p.IsRunning() {
761+
t.Error("expected IsRunning() == true after Start()")
762+
}
763+
p.Stop()
764+
if p.IsRunning() {
765+
t.Error("expected IsRunning() == false after Stop()")
766+
}
767+
}
768+
769+
func TestStop_ResetsState(t *testing.T) {
770+
p := NewScalametaParser()
771+
if err := p.Start(); err != nil {
772+
t.Fatal("Start:", err)
773+
}
774+
if p.httpPort == 0 {
775+
t.Error("expected non-zero httpPort after Start()")
776+
}
777+
if p.httpUrl == "" {
778+
t.Error("expected non-empty httpUrl after Start()")
779+
}
780+
p.Stop()
781+
if p.httpPort != 0 {
782+
t.Errorf("expected httpPort == 0 after Stop(), got %d", p.httpPort)
783+
}
784+
if p.httpUrl != "" {
785+
t.Errorf("expected httpUrl == \"\" after Stop(), got %q", p.httpUrl)
786+
}
787+
}
788+
789+
func TestRestart(t *testing.T) {
790+
p := NewScalametaParser()
791+
792+
if err := p.Start(); err != nil {
793+
t.Fatal("first Start:", err)
794+
}
795+
if !p.IsRunning() {
796+
t.Fatal("expected IsRunning() after first Start()")
797+
}
798+
799+
p.Stop()
800+
if p.IsRunning() {
801+
t.Fatal("expected !IsRunning() after Stop()")
802+
}
803+
804+
if err := p.Start(); err != nil {
805+
t.Fatal("second Start:", err)
806+
}
807+
if !p.IsRunning() {
808+
t.Error("expected IsRunning() after restart")
809+
}
810+
811+
// Verify the restarted parser can actually serve requests
812+
response, err := p.Parse(context.Background(), &sppb.ParseRequest{})
813+
if err != nil {
814+
t.Fatal("Parse after restart:", err)
815+
}
816+
if response.Error == "" {
817+
t.Error("expected error response for empty request")
818+
}
819+
820+
p.Stop()
821+
}
822+
748823
func mustWriteTestFiles(t *testing.T, tmpDir string, files []testtools.FileSpec) []string {
749824
var filenames []string
750825
for _, file := range files {

rules/java_indexer_aspect.bzl

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,6 @@ COMPILE_TIME = 0
6060

6161
RUNTIME = 1
6262

63-
DEFAULT_REPO_NAME_SEPARATOR = "+"
64-
6563
# Compile-time dependency attributes, grouped by type.
6664
DEPS = [
6765
"_cc_toolchain", # From cc rules
@@ -246,11 +244,26 @@ def _jarindex_basename(ctx, label):
246244

247245
# see https://bazelbuild.slack.com/archives/C014RARENH0/p1752851984151199?thread_ts=1752594227.746349&cid=C014RARENH0 - we can't get the label apparent name! ("@maven")
248246
def get_apparent_label(label):
249-
parts = label.repo_name.split(DEFAULT_REPO_NAME_SEPARATOR)
250-
apparent_name = parts[len(parts) - 1]
251-
apparent_label = "@%s//%s:%s" % (apparent_name, label.package, label.name)
247+
"""Returns the apparent label string for a bzlmod canonical label.
248+
249+
Splits the canonical repo name on '+' (Bazel 8+) or '~' (Bazel 7) to
250+
extract the apparent repo name (e.g. 'maven' from 'rules_jvm_external+5.3+maven').
251+
252+
Args:
253+
label: A Label object.
252254

253-
return apparent_label
255+
Returns:
256+
A label string using the apparent repo name, e.g. "@maven//pkg:target".
257+
"""
258+
repo_name = label.repo_name
259+
if "+" in repo_name:
260+
parts = repo_name.split("+")
261+
elif "~" in repo_name:
262+
parts = repo_name.split("~")
263+
else:
264+
parts = [repo_name]
265+
apparent_name = parts[len(parts) - 1]
266+
return "@%s//%s:%s" % (apparent_name, label.package, label.name)
254267

255268
def jarindexer_action(ctx, label, kind, executable, jar):
256269
output_file = ctx.actions.declare_file(_jarindex_basename(ctx, label) + ".javaindex.pb")

0 commit comments

Comments
 (0)