Skip to content
Merged
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
23 changes: 12 additions & 11 deletions internal/librarian/library.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/googleapis/librarian/internal/config"
"github.com/googleapis/librarian/internal/librarian/golang"
"github.com/googleapis/librarian/internal/librarian/java"
"github.com/googleapis/librarian/internal/librarian/nodejs"
"github.com/googleapis/librarian/internal/librarian/python"
"github.com/googleapis/librarian/internal/librarian/rust"
"github.com/googleapis/librarian/internal/librarian/swift"
Expand Down Expand Up @@ -193,16 +194,16 @@ func mergePackageDependencies(defaults, lib []*config.RustPackageDependency) []*
return result
}

// isVeneer reports whether the library has handwritten code wrapping generated
// code.
func isVeneer(language string, lib *config.Library) bool {
// isMixedLibrary reports whether the library is composed of both handwritten
// and librarian-generated code.
func isMixedLibrary(language string, lib *config.Library) bool {
switch language {
case config.LanguageRust:
return rust.IsVeneer(lib)
return rust.IsMixedLibrary(lib)
case config.LanguageSwift:
return swift.IsModule(lib)
return swift.IsMixedLibrary(lib)
case config.LanguageNodejs:
return lib.Output != "" && len(lib.APIs) == 0
return nodejs.IsMixedLibrary(lib)
default:
return false
}
Expand All @@ -215,8 +216,8 @@ func libraryOutput(language string, lib *config.Library, defaults *config.Defaul
if lib.Output != "" {
return lib.Output
}
if isVeneer(language, lib) {
// Veneers require explicit output, so return empty if not set.
if isMixedLibrary(language, lib) {
// Mixed or non-generated libraries require explicit output, so return empty if not set.
return ""
}
apiPath := deriveAPIPath(language, lib.Name)
Expand All @@ -232,7 +233,7 @@ func libraryOutput(language string, lib *config.Library, defaults *config.Defaul

// applyDefaults applies language-specific derivations and fills defaults.
func applyDefaults(language string, lib *config.Library, defaults *config.Default) (*config.Library, error) {
if !isVeneer(language, lib) {
if !isMixedLibrary(language, lib) {
if len(lib.APIs) == 0 && canDeriveAPIPath(language) {
// Do not derive API path for some languages because the library
// name doesn't contain all the required info.
Expand All @@ -245,8 +246,8 @@ func applyDefaults(language string, lib *config.Library, defaults *config.Defaul
}
}
if lib.Output == "" {
if isVeneer(language, lib) {
return nil, fmt.Errorf("veneer %q requires an explicit output path", lib.Name)
if isMixedLibrary(language, lib) {
Comment thread
noahdietz marked this conversation as resolved.
return nil, fmt.Errorf("library %q requires an explicit output path", lib.Name)
}
var apiPath string
if len(lib.APIs) > 0 {
Expand Down
14 changes: 7 additions & 7 deletions internal/librarian/library_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -644,15 +644,15 @@ func TestCanDeriveAPIPath(t *testing.T) {
}
}

func TestIsVeneer(t *testing.T) {
func TestIsMixedLibrary(t *testing.T) {
for _, test := range []struct {
name string
language string
lib *config.Library
want bool
}{
{
name: "rust is veneer",
name: "rust is mixed library",
language: config.LanguageRust,
lib: &config.Library{
Rust: &config.RustCrate{
Expand All @@ -662,13 +662,13 @@ func TestIsVeneer(t *testing.T) {
want: true,
},
{
name: "rust is not veneer",
name: "rust is not mixed library",
language: config.LanguageRust,
lib: &config.Library{},
want: false,
},
{
name: "nodejs handwritten tool is veneer",
name: "nodejs handwritten tool is mixed library",
language: config.LanguageNodejs,
lib: &config.Library{
Output: "packages/typeless-sample-bot",
Expand All @@ -677,7 +677,7 @@ func TestIsVeneer(t *testing.T) {
want: true,
},
{
name: "nodejs gapic lib is not veneer",
name: "nodejs gapic lib is not mixed library",
language: config.LanguageNodejs,
lib: &config.Library{
Output: "packages/gapic-lib",
Expand All @@ -687,8 +687,8 @@ func TestIsVeneer(t *testing.T) {
},
} {
t.Run(test.name, func(t *testing.T) {
if got := isVeneer(test.language, test.lib); got != test.want {
t.Errorf("isVeneer(%q, %+v) = %v, want %v", test.language, test.lib, got, test.want)
if got := isMixedLibrary(test.language, test.lib); got != test.want {
t.Errorf("isMixedLibrary(%q, %+v) = %v, want %v", test.language, test.lib, got, test.want)
}
})
}
Expand Down
6 changes: 6 additions & 0 deletions internal/librarian/nodejs/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ const (
commonProtos = "google/cloud/common_resources.proto"
)

// IsMixedLibrary reports whether the library has handwritten code wrapping
// generated or librarian-managed code.
func IsMixedLibrary(lib *config.Library) bool {
return lib.Output != "" && len(lib.APIs) == 0
}

// Generate generates a Node.js client library.
func Generate(ctx context.Context, cfg *config.Config, library *config.Library, srcs *sources.Sources) error {
googleapisDir := srcs.Googleapis
Expand Down
39 changes: 39 additions & 0 deletions internal/librarian/nodejs/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,45 @@ import (

const googleapisDir = "../../testdata/googleapis"

func TestIsMixedLibrary(t *testing.T) {
for _, test := range []struct {
name string
lib *config.Library
want bool
}{
{
name: "mixed library case",
lib: &config.Library{
Output: "packages/typeless-sample-bot",
APIs: nil,
},
want: true,
},
{
name: "standard gapic lib",
lib: &config.Library{
Output: "packages/gapic-lib",
APIs: []*config.API{{Path: "google/example/v1"}},
},
want: false,
},
{
name: "no output set",
lib: &config.Library{
Output: "",
APIs: nil,
},
want: false,
},
} {
t.Run(test.name, func(t *testing.T) {
if got := IsMixedLibrary(test.lib); got != test.want {
t.Errorf("IsMixedLibrary() = %v, want %v", got, test.want)
}
})
}
}

func TestDerivePackageName(t *testing.T) {
for _, test := range []struct {
name string
Expand Down
22 changes: 11 additions & 11 deletions internal/librarian/rust/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ import (
"github.com/googleapis/librarian/internal/sources"
)

// IsVeneer reports whether the library has handwritten code wrapping generated
// code.
// IsMixedLibrary reports whether the library has handwritten code wrapping
// generated code.
//
// A library is a veneer when it has Rust module configuration. A library with
// no APIs and an explicit output is a veneer if its derived API path is not
// listed in sdk.yaml; libraries whose derived path appears in sdk.yaml are
// generated libraries whose APIs have not been populated yet (e.g.
// google-cloud-oslogin-common), not veneers.
func IsVeneer(lib *config.Library) bool {
// A library is a mixed library when it has Rust module configuration. A library
// with no APIs and an explicit output is a mixed library if its derived API
// path is not listed in sdk.yaml; libraries whose derived path appears in
// sdk.yaml are generated libraries whose APIs have not yet been populated
// (e.g. google-cloud-oslogin-common), not mixed libraries.
func IsMixedLibrary(lib *config.Library) bool {
if lib.Rust != nil && len(lib.Rust.Modules) > 0 {
return true
}
if len(lib.APIs) == 0 && lib.Output != "" {
// If the derived API path is in sdk.yaml, this is a generated
// library whose APIs have not been populated yet, not a veneer.
// library whose APIs have not yet been populated, not a mixed library.
if serviceconfig.HasAPIPath(DeriveAPIPath(lib.Name), config.LanguageRust) {
return false
}
Expand All @@ -58,7 +58,7 @@ func IsVeneer(lib *config.Library) bool {

// Generate generates a Rust client library.
func Generate(ctx context.Context, cfg *config.Config, library *config.Library, sources *sources.Sources) error {
if IsVeneer(library) {
if IsMixedLibrary(library) {
return generateVeneer(ctx, library, sources)
}
if len(library.APIs) != 1 {
Expand Down Expand Up @@ -168,7 +168,7 @@ func generateVeneer(ctx context.Context, library *config.Library, sources *sourc

// Keep returns the list of files to preserve when cleaning the output directory.
func Keep(library *config.Library) ([]string, error) {
if !IsVeneer(library) {
if !IsMixedLibrary(library) {
Comment thread
noahdietz marked this conversation as resolved.
return library.Keep, nil
}
// For veneers, keep all files outside module output directories. We walk
Expand Down
6 changes: 3 additions & 3 deletions internal/librarian/rust/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func TestGenerateVeneer(t *testing.T) {
}
}

func TestIsVeneer(t *testing.T) {
func TestIsMixedLibrary(t *testing.T) {
for _, test := range []struct {
name string
lib *config.Library
Expand Down Expand Up @@ -158,8 +158,8 @@ func TestIsVeneer(t *testing.T) {
},
} {
t.Run(test.name, func(t *testing.T) {
if got := IsVeneer(test.lib); got != test.want {
t.Errorf("IsVeneer() = %v, want %v", got, test.want)
if got := IsMixedLibrary(test.lib); got != test.want {
t.Errorf("IsMixedLibrary() = %v, want %v", got, test.want)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/librarian/swift/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import (

// Generate generates a Swift client library.
func Generate(ctx context.Context, cfg *config.Config, library *config.Library, src *sources.Sources) error {
if IsModule(library) {
if IsMixedLibrary(library) {
return generateModule(ctx, library, src)
}
if len(library.APIs) != 1 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ package swift

import "github.com/googleapis/librarian/internal/config"

// IsModule reports whether the library has handwritten code wrapping generated code.
// IsMixedLibrary reports whether the library has handwritten code wrapping generated code.
Comment thread
noahdietz marked this conversation as resolved.
//
// At the moment, only libraries created for test protos are modules. Such libraries do not have an API and have an
// explicit output directory.
func IsModule(lib *config.Library) bool {
func IsMixedLibrary(lib *config.Library) bool {
return lib.Swift != nil && len(lib.Swift.Modules) > 0
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,18 @@ import (
"github.com/googleapis/librarian/internal/config"
)

func TestIsModule(t *testing.T) {
func TestIsMixedLibrary(t *testing.T) {
for _, test := range []struct {
name string
lib *config.Library
want bool
}{
{"module", &config.Library{Swift: &config.SwiftPackage{Modules: []*config.SwiftModule{{}}}}, true},
{"mixed library", &config.Library{Swift: &config.SwiftPackage{Modules: []*config.SwiftModule{{}}}}, true},
{"nil swift", &config.Library{}, false},
{"empty modules", &config.Library{Swift: &config.SwiftPackage{}}, false},
{"fully generated library", &config.Library{Swift: &config.SwiftPackage{}}, false},
} {
t.Run(test.name, func(t *testing.T) {
got := IsModule(test.lib)
got := IsMixedLibrary(test.lib)
if diff := cmp.Diff(test.want, got); diff != "" {
t.Errorf("mismatch (-want +got):\n%s", diff)
}
Expand Down
5 changes: 3 additions & 2 deletions internal/librarian/tidy.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ func tidyLibrary(cfg *config.Config, lib *config.Library) *config.Library {
if lib.Output != "" && len(lib.APIs) == 1 && isDerivableOutput(cfg, lib) {
lib.Output = ""
}
if isVeneer(cfg.Language, lib) {
// Veneers are never generated, so ensure skip_generate is false.
if isMixedLibrary(cfg.Language, lib) {
// Mixed libraries with handwritten code or those that are fully handwritten
// are never generated, so ensure that skip_generate is false.
Comment thread
noahdietz marked this conversation as resolved.
lib.SkipGenerate = false
}
if len(lib.Roots) == 1 && lib.Roots[0] == "googleapis" {
Expand Down
Loading