-
Notifications
You must be signed in to change notification settings - Fork 32
feat: the languagecontainer module to share data and functions with language containers #2489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
94d4477
c632576
3e0d668
06b8d33
91a8370
b06d54e
4fb911c
ec4c803
1e6cc25
caeeb19
ef131ab
653fb85
9845ffa
c0f6ba7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package bazel provides functions to parse Bazel-related files. | ||
| package bazel | ||
|
|
||
| // TODO: Implement this package. Reference https://github.com/googleapis/google-cloud-go/tree/main/internal/librariangen/bazel | ||
| // and Mike's porting for Java. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| // Package cmd provides the main function LanguageContainerMain that | ||
| // handles command line argument parsing and invocation of the corresponding methods. | ||
| package cmd | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think all of this code should be moved up a directory. Or at the very least this should have a different name. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, let's do that. I like |
||
|
|
||
| import ( | ||
| "context" | ||
| ) | ||
|
|
||
| // GenerateFlags is the flags in Librarian's container contract for the | ||
| // generate command. Each value represents the path of the context, such as | ||
| // "/librarian", in which the CLI and the language container exchange | ||
| // generate-request.json and generate-response.json. | ||
| // https://github.com/googleapis/librarian/blob/main/doc/language-onboarding.md#generate | ||
| type GenerateFlags struct { | ||
| Librarian string | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add godoc to an public struct fields. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me focus on the package organization for now. |
||
| Input string | ||
| Output string | ||
| Source string | ||
| } | ||
|
|
||
| // ConfigureFlags holds flags for the `configure` command. | ||
| // https://github.com/googleapis/librarian/blob/main/doc/language-onboarding.md#configure | ||
| type ConfigureFlags struct { | ||
| Librarian string | ||
| Input string | ||
| Repo string | ||
| Source string | ||
| Output string | ||
| } | ||
|
|
||
| // ReleaseInitFlags holds flags for the `release-init` command. | ||
| // https://github.com/googleapis/librarian/blob/main/doc/language-onboarding.md#release-init | ||
| type ReleaseInitFlags struct { | ||
| Librarian string | ||
| Repo string | ||
| Output string | ||
| } | ||
|
|
||
| // LanguageContainerFunctions lists the functions a language container | ||
| // defines to accept requests from Librarian CLI. | ||
| type LanguageContainerFunctions struct { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I would just call this LanguageContainer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and or move all of these things up a directory and call Functions. That would give usage the look: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a great idea. I added that in go/sdk-librarian-containers-in-go?tab=t.ft3dwbukyu66 as |
||
| Generate func(ctx context.Context, flags *GenerateFlags) | ||
| Configure func(ctx context.Context, flags *ConfigureFlags) | ||
| ReleaseInit func(ctx context.Context, flags *ReleaseInitFlags) | ||
| } | ||
|
|
||
| // Entry point for a language container. This parses the arguments and | ||
| // calls the corresponding function in the functions struct. | ||
| // | ||
| // A language container defines the following main function: | ||
| // | ||
| // ``` | ||
| // | ||
| // func main() { | ||
| // cmd.LanguageContainerMain(os.Args, cmd.LanguageContainerFunctions{ ... }) | ||
| // } | ||
| // | ||
| // ``` | ||
| func LanguageContainerMain(args []string, functions LanguageContainerFunctions) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I took that idea in go/sdk-librarian-containers-in-go?tab=t.ft3dwbukyu66. |
||
| // TODO: Parse the arguments correctly. | ||
| if len(args) > 0 && args[0] == "generate" && functions.Generate != nil { | ||
| generateFlags := GenerateFlags{ | ||
| Librarian: "/librarian", | ||
| Input: "/input", | ||
| Output: "/output", | ||
| Source: "/source", | ||
| } | ||
| functions.Generate(context.Background(), &generateFlags) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,50 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // https://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package cmd | ||
|
|
||
| import ( | ||
| "context" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestLanguageContainerMain_ParseArguments(t *testing.T) { | ||
| var calledFunction string | ||
| for _, test := range []struct { | ||
| name string | ||
| arguments []string | ||
| functions LanguageContainerFunctions | ||
| }{ | ||
| { | ||
| name: "parse generate command", | ||
| arguments: []string{ | ||
| "generate", | ||
| }, | ||
| functions: LanguageContainerFunctions{ | ||
| Generate: func(ctx context.Context, flags *GenerateFlags) { | ||
| calledFunction = "generate" | ||
| }, | ||
| }, | ||
| }, | ||
| } { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| calledFunction = "None" | ||
| LanguageContainerMain(test.arguments, test.functions) | ||
| if calledFunction != test.arguments[0] { | ||
| t.Errorf("LanguageContainerMain didn't call the correct function. Expected %s, got %s", | ||
| test.arguments[0], calledFunction) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package execv | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the API for such a package should just mimic https://pkg.go.dev/os/exec#Cmd. It would be nice it was a drop in replacement for the stdlib -- API wise. |
||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "log/slog" | ||
| "os" | ||
| "os/exec" | ||
| "strings" | ||
| ) | ||
|
|
||
| // Run executes a command in a specified working directory and logs its output. | ||
| func Run(ctx context.Context, args []string, workingDir string) error { | ||
| cmd := exec.CommandContext(ctx, args[0], args[1:]...) | ||
| cmd.Env = os.Environ() | ||
| cmd.Dir = workingDir | ||
| slog.Debug("librariangen: running command", "command", strings.Join(cmd.Args, " "), "dir", cmd.Dir) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove all references to librariangen here. This is an idiom usually only used in error msgs and signals where an error came from. For logging you can see the source line if that feature is turned on. ( It is off by default) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's forget about this execv package for now to focus on the package organization. |
||
|
|
||
| output, err := cmd.Output() | ||
| if len(output) > 0 { | ||
| slog.Debug("librariangen: command stdout", "output", string(output)) | ||
| } | ||
| if err != nil { | ||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| // The command ran and exited with a non-zero exit code. | ||
| if len(exitErr.Stderr) > 0 { | ||
| slog.Debug("librariangen: command stderr", "output", string(exitErr.Stderr)) | ||
| } | ||
| return fmt.Errorf("librariangen: command failed with exit error: %s: %w", exitErr.Stderr, err) | ||
| } | ||
| // Another error occurred (e.g., command not found). | ||
| return fmt.Errorf("librariangen: command failed: %w", err) | ||
| } | ||
| return nil | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| // Copyright 2025 Google LLC | ||
| // | ||
| // Licensed under the Apache License, Version 2.0 (the "License"); | ||
| // you may not use this file except in compliance with the License. | ||
| // You may obtain a copy of the License at | ||
| // | ||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||
| // | ||
| // Unless required by applicable law or agreed to in writing, software | ||
| // distributed under the License is distributed on an "AS IS" BASIS, | ||
| // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| // See the License for the specific language governing permissions and | ||
| // limitations under the License. | ||
|
|
||
| package execv | ||
|
|
||
| import ( | ||
| "context" | ||
| "errors" | ||
| "os/exec" | ||
| "strings" | ||
| "testing" | ||
| ) | ||
|
|
||
| func TestRun(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| args []string | ||
| wantErr bool | ||
| wantExit int | ||
| wantInStderr string | ||
| }{ | ||
| { | ||
| name: "valid command", | ||
| args: []string{"echo", "hello"}, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "invalid command", | ||
| args: []string{"command-that-does-not-exist"}, | ||
| wantErr: true, | ||
| }, | ||
| { | ||
| name: "command with non-zero exit", | ||
| args: []string{"sh", "-c", "exit 1"}, | ||
| wantErr: true, | ||
| wantExit: 1, | ||
| }, | ||
| { | ||
| name: "command with stderr output", | ||
| args: []string{"sh", "-c", "echo 'test error' >&2; exit 1"}, | ||
| wantErr: true, | ||
| wantExit: 1, | ||
| wantInStderr: "test error", | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| err := Run(context.Background(), tt.args, ".") | ||
| if (err != nil) != tt.wantErr { | ||
| t.Fatalf("Run() error = %v, wantErr %v", err, tt.wantErr) | ||
| } | ||
|
|
||
| if !tt.wantErr { | ||
| return | ||
| } | ||
|
|
||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| if tt.wantExit != 0 && exitErr.ExitCode() != tt.wantExit { | ||
| t.Errorf("Run() exit code = %d, want %d", exitErr.ExitCode(), tt.wantExit) | ||
| } | ||
| if tt.wantInStderr != "" && !strings.Contains(string(exitErr.Stderr), tt.wantInStderr) { | ||
| t.Errorf("Run() stderr = %q, want contains %q", string(exitErr.Stderr), tt.wantInStderr) | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| // Module languagecontainer provides packages to be shared | ||
| // with language container implementations. | ||
| module github.com/googleapis/librarian/languagecontainer | ||
|
|
||
| go 1.25.0 | ||
|
|
||
| require github.com/googleapis/librarian v0.0.0 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should try to avoid depending on the other module if possibile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. With the comment you wrote for the "message" package, I'm now thinking of removing this module and having the language containers to depend on the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's now in go/sdk-librarian-containers-in-go?tab=t.ft3dwbukyu66 |
||
|
|
||
| replace github.com/googleapis/librarian => ../ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would avoid putting replace statement in a module other ppl will depend on. It can create cases where things will build here locally with CI but fail for ppl that depend on the library. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm now thinking of using the root librarian module for sharing with language containers. PTAL go/sdk-librarian-containers-in-go?tab=t.ft3dwbukyu66 |
||
|
|
||
| require ( | ||
| dario.cat/mergo v1.0.2 // indirect | ||
| github.com/Microsoft/go-winio v0.6.2 // indirect | ||
| github.com/ProtonMail/go-crypto v1.3.0 // indirect | ||
| github.com/cloudflare/circl v1.6.1 // indirect | ||
| github.com/cyphar/filepath-securejoin v0.4.1 // indirect | ||
| github.com/emirpasic/gods v1.18.1 // indirect | ||
| github.com/go-git/gcfg v1.5.1-0.20230307220236-3a3c6141e376 // indirect | ||
| github.com/go-git/go-billy/v5 v5.6.2 // indirect | ||
| github.com/go-git/go-git/v5 v5.16.2 // indirect | ||
| github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect | ||
| github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect | ||
| github.com/kevinburke/ssh_config v1.4.0 // indirect | ||
| github.com/klauspost/cpuid/v2 v2.3.0 // indirect | ||
| github.com/pjbgf/sha1cd v0.5.0 // indirect | ||
| github.com/sergi/go-diff v1.4.0 // indirect | ||
| github.com/skeema/knownhosts v1.3.1 // indirect | ||
| github.com/xanzy/ssh-agent v0.3.3 // indirect | ||
| golang.org/x/crypto v0.42.0 // indirect | ||
| golang.org/x/net v0.44.0 // indirect | ||
| golang.org/x/sys v0.36.0 // indirect | ||
| gopkg.in/warnings.v0 v0.1.2 // indirect | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: don't add an empty package. We can add later when we know what we want this to look like.