Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions all_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ var noHeaderRequiredFiles = []string{
".gitignore",
"Dockerfile",
"LICENSE",
"languagecontainer/go.mod",
"languagecontainer/go.sum",
"coverage.out",
"go.mod",
"go.sum",
Expand Down
19 changes: 19 additions & 0 deletions languagecontainer/bazel/parser.go
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
Copy link
Member

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.

// and Mike's porting for Java.
84 changes: 84 additions & 0 deletions languagecontainer/cmd/cmd.go
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
Copy link
Member

Choose a reason for hiding this comment

The 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. cmd has meaning in Go project to represent the main package of CLIs. See the root package in this project as an example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's do that. I like languagecontainer.Run idea. go/sdk-librarian-containers-in-go?tab=t.ft3dwbukyu66


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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add godoc to an public struct fields.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I would just call this LanguageContainer

Copy link
Member

Choose a reason for hiding this comment

The 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: languagecontainer.Functions in calling code.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 languagecontainer.Functions.

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) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Run. If you like the other feedback about that would make it languagecontainer.Run(...)

Copy link
Member Author

Choose a reason for hiding this comment

The 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)
}
}
50 changes: 50 additions & 0 deletions languagecontainer/cmd/cmd_test.go
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)
}
})
}
}
51 changes: 51 additions & 0 deletions languagecontainer/execv/execv.go
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
Copy link
Member

Choose a reason for hiding this comment

The 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)
Copy link
Member

Choose a reason for hiding this comment

The 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)

Copy link
Member Author

Choose a reason for hiding this comment

The 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
}
79 changes: 79 additions & 0 deletions languagecontainer/execv/execv_test.go
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)
}
}
})
}
}
33 changes: 33 additions & 0 deletions languagecontainer/go.mod
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
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

@suztomo suztomo Oct 10, 2025

Choose a reason for hiding this comment

The 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 github.com/googleapis/librarian module. The "languagecontainer/*" are packages inside the module that they use for language-container specific work.

Copy link
Member Author

Choose a reason for hiding this comment

The 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 => ../
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

Choose a reason for hiding this comment

The 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
)
Loading