Skip to content

Commit e33192f

Browse files
sofislsofisl
authored
feat(internal/tool/composer): use entrypoint configuration instead of package names (#7156)
This change replaces the hardcoded `gapic-generator-php` package name checks in composer tool installation with a declarative `entrypoint` configuration field. If a tool defines an `entrypoint` (e.g. `src/Main.php`), a bash wrapper script is generated for it in the bin directory. If the `entrypoint` is omitted, no wrapper is generated. This refactoring prepares the composer installation logic to generically support PHP tools that require different entrypoints, or tools that act strictly as dependencies and do not require executable wrappers. ### Merge Order & PR Relationship This PR is the foundational step of a three-part refactoring effort and should be merged **first**: 1. **Merge this PR first:** It introduces the generic `Entrypoint` logic and removes the hardcoded checks that previously blocked non-generator tools from being installed. 2. **Merge PR #7122 second:** With this PR merged, PR #7122 can cleanly install `google-cloud-php/dev`. Because `google-cloud-php/dev` will not specify an `entrypoint`, Librarian will safely run `composer install` on it without attempting (and failing) to generate an executable wrapper for it. 3. **Merge PR #7141 third:** Now that wrapper generation is decoupled from the `gapic-generator-php` package name, PR #7141 can safely remove the hardcoded `--side_loaded_root_dir` flag from the wrapper script template. This will make the PHP wrapper logic fully generic and reusable for any future tool.
1 parent 17f5a3d commit e33192f

7 files changed

Lines changed: 132 additions & 53 deletions

File tree

‎doc/config-schema.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ This document describes the schema for the librarian.yaml.
6262
| `repo` | string | Is the GitHub repository to fetch the tool from (e.g. github.com/googleapis/gapic-generator-php). |
6363
| `sha256` | string | Is the SHA256 checksum of the package. |
6464
| `local_path` | string | Is the path to a local composer project. When present, Version, Repo, and SHA256 are ignored. |
65+
| `entrypoint` | string | Is the path to the main script to execute. If set, an executable wrapper is generated for this tool. |
6566

6667
## GemTool Configuration
6768

‎internal/config/config.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ type ComposerTool struct {
145145
// LocalPath is the path to a local composer project.
146146
// When present, Version, Repo, and SHA256 are ignored.
147147
LocalPath string `yaml:"local_path,omitempty"`
148+
149+
// Entrypoint is the path to the main script to execute.
150+
// If set, an executable wrapper is generated for this tool.
151+
Entrypoint string `yaml:"entrypoint,omitempty"`
148152
}
149153

150154
// GemTool defines a tool to install via gem.

‎internal/librarian/php/install_test.go‎

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,11 @@ func TestInstall(t *testing.T) {
6666
tools: &config.Tools{
6767
Composer: []*config.ComposerTool{
6868
{
69-
Name: "gapic-generator-php",
70-
Version: "1.0.0",
71-
Repo: "github.com/googleapis/gapic-generator-php",
72-
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
69+
Name: "gapic-generator-php",
70+
Entrypoint: "src/Main.php",
71+
Version: "1.0.0",
72+
Repo: "github.com/googleapis/gapic-generator-php",
73+
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
7374
},
7475
},
7576
Pip: []*config.PipTool{
@@ -93,6 +94,9 @@ func TestInstall(t *testing.T) {
9394
if err := os.MkdirAll(filepath.Join(repoDir, "dummy"), 0o755); err != nil {
9495
t.Fatal(err)
9596
}
97+
if err := os.WriteFile(filepath.Join(repoDir, "composer.json"), []byte("{}"), 0o644); err != nil {
98+
t.Fatal(err)
99+
}
96100

97101
bin := t.TempDir()
98102
testhelper.WriteExecutable(t, filepath.Join(bin, "composer"), "#!/bin/sh\nexit 0\n")
@@ -139,8 +143,9 @@ func TestInstall_Error(t *testing.T) {
139143
tools: &config.Tools{
140144
Composer: []*config.ComposerTool{
141145
{
142-
Name: "gapic-generator-php",
143-
Version: "1.0.0",
146+
Name: "gapic-generator-php",
147+
Entrypoint: "src/Main.php",
148+
Version: "1.0.0",
144149
},
145150
},
146151
Pip: []*config.PipTool{
@@ -180,9 +185,10 @@ func TestInstall_Error(t *testing.T) {
180185
tools: &config.Tools{
181186
Composer: []*config.ComposerTool{
182187
{
183-
Name: "gapic-generator-php",
184-
Version: "1.0.0",
185-
Repo: "github.com/googleapis/gapic-generator-php",
188+
Name: "gapic-generator-php",
189+
Entrypoint: "src/Main.php",
190+
Version: "1.0.0",
191+
Repo: "github.com/googleapis/gapic-generator-php",
186192
},
187193
},
188194
},
@@ -193,9 +199,10 @@ func TestInstall_Error(t *testing.T) {
193199
tools: &config.Tools{
194200
Composer: []*config.ComposerTool{
195201
{
196-
Name: "gapic-generator-php",
197-
Version: "1.0.0",
198-
Repo: "github.com/googleapis/gapic-generator-php",
202+
Name: "gapic-generator-php",
203+
Entrypoint: "src/Main.php",
204+
Version: "1.0.0",
205+
Repo: "github.com/googleapis/gapic-generator-php",
199206
},
200207
},
201208
Pip: []*config.PipTool{
@@ -212,9 +219,10 @@ func TestInstall_Error(t *testing.T) {
212219
tools: &config.Tools{
213220
Composer: []*config.ComposerTool{
214221
{
215-
Name: "gapic-generator-php",
216-
Version: "1.0.0",
217-
Repo: "github.com/googleapis/gapic-generator-php",
222+
Name: "gapic-generator-php",
223+
Entrypoint: "src/Main.php",
224+
Version: "1.0.0",
225+
Repo: "github.com/googleapis/gapic-generator-php",
218226
},
219227
},
220228
Pip: []*config.PipTool{
@@ -238,6 +246,9 @@ func TestInstall_Error(t *testing.T) {
238246
if err := os.MkdirAll(filepath.Join(repoDir, "dummy"), 0o755); err != nil {
239247
t.Fatal(err)
240248
}
249+
if err := os.WriteFile(filepath.Join(repoDir, "composer.json"), []byte("{}"), 0o644); err != nil {
250+
t.Fatal(err)
251+
}
241252
t.Setenv("PATH", t.TempDir())
242253
},
243254
wantErr: exec.ErrNotFound,

‎internal/tool/composer/composer.go‎

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"os"
2323
"path/filepath"
24+
"strings"
2425

2526
"github.com/googleapis/librarian/internal/command"
2627
"github.com/googleapis/librarian/internal/config"
@@ -36,6 +37,7 @@ var (
3637
)
3738

3839
// Install installs a list of Composer tools into the environment.
40+
// It also installs dependencies for the PHP project if a local_path tool (like "dev") is provided.
3941
func Install(ctx context.Context, tools []*config.ComposerTool, phpPath, bin string) error {
4042
if err := verify(tools); err != nil {
4143
return err
@@ -57,23 +59,14 @@ func Install(ctx context.Context, tools []*config.ComposerTool, phpPath, bin str
5759
if err := command.RunInDir(ctx, dir, "composer", "install", "--no-interaction", "--prefer-dist"); err != nil {
5860
return fmt.Errorf("failed to run composer install: %w", err)
5961
}
62+
if tool.Entrypoint == "" {
63+
continue // No wrapper needed
64+
}
6065
wrapperName := filepath.Base(tool.Name)
61-
if wrapperName == "gapic-generator-php" {
62-
// Currently, this assumes the tool is the gapic-generator-php. This specific
63-
// wrapper logic will not work for generic Composer tools because:
64-
// 1. It hardcodes the executable entry point to "src/Main.php" (ignoring Composer's vendor/bin/ paths).
65-
// 2. It injects specific PHP configurations (e.g. memory_limit=1024M) required to prevent the generator from crashing.
66-
// See https://github.com/googleapis/gapic-generator-php/commit/685b419f2220e2d19c74e7f1464067f995cf1a95
67-
// 3. It automatically injects the "--side_loaded_root_dir" argument which other tools will not expect.
68-
// (this argument is to pass through relative paths for config files)
69-
// TODO(https://github.com/googleapis/librarian/issues/7000): Remove the --side_loaded_root_dir once we pass full paths to generator
70-
destPath := filepath.Join(dir, "src", "Main.php")
71-
wrapperContent := phpWrapperContent(phpPath, destPath)
72-
if err := createBinWrapper(wrapperName, wrapperContent, bin); err != nil {
73-
return err
74-
}
75-
} else {
76-
return fmt.Errorf("tool installation for non-generator composer tools is not yet supported")
66+
destPath := filepath.Join(dir, tool.Entrypoint)
67+
wrapperContent := phpWrapperContent(phpPath, destPath)
68+
if err := createBinWrapper(wrapperName, wrapperContent, bin); err != nil {
69+
return err
7770
}
7871
}
7972
return nil
@@ -86,9 +79,6 @@ func localPath(path string) (string, error) {
8679
return "", fmt.Errorf("failed to resolve absolute path for %s: %w", path, err)
8780
}
8881
if _, err := os.Stat(absPath); err != nil {
89-
if errors.Is(err, os.ErrNotExist) {
90-
return "", fmt.Errorf("local composer path not found: %w", err)
91-
}
9282
return "", fmt.Errorf("failed to stat local composer path: %w", err)
9383
}
9484
return absPath, nil
@@ -117,6 +107,9 @@ func verify(tools []*config.ComposerTool) error {
117107
if tool.Name == "" {
118108
return fmt.Errorf("%w: name must be specified: %+v", ErrInvalidTool, tool)
119109
}
110+
if filepath.IsAbs(tool.Entrypoint) || strings.Contains(tool.Entrypoint, "..") {
111+
return fmt.Errorf("%w: entrypoint must be a clean relative path: %+v", ErrInvalidTool, tool)
112+
}
120113
hasLocal := tool.LocalPath != ""
121114
hasRemote := tool.Version != "" || tool.Repo != "" || tool.SHA256 != ""
122115
if hasLocal && hasRemote {

‎internal/tool/composer/composer_test.go‎

Lines changed: 83 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package composer
1717
import (
1818
"errors"
1919
"fmt"
20+
"io/fs"
2021
"os"
2122
"path/filepath"
2223
"testing"
@@ -34,20 +35,24 @@ func TestInstall(t *testing.T) {
3435
if err := os.MkdirAll(filepath.Join(repoDir, "dummy"), 0o755); err != nil {
3536
t.Fatal(err)
3637
}
38+
if err := os.WriteFile(filepath.Join(repoDir, "composer.json"), []byte("{}"), 0o644); err != nil {
39+
t.Fatal(err)
40+
}
3741
bin := t.TempDir()
3842
testhelper.WriteExecutable(t, filepath.Join(bin, "composer"), "#!/bin/sh\nexit 0\n")
3943
t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
4044
binDir := t.TempDir()
4145
tools := []*config.ComposerTool{
4246
{
43-
Name: "gapic-generator-php",
44-
Version: "1.0.0",
45-
Repo: "github.com/googleapis/gapic-generator-php",
46-
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
47+
Name: "gapic-generator-php",
48+
Version: "1.0.0",
49+
Repo: "github.com/googleapis/gapic-generator-php",
50+
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
51+
Entrypoint: "src/Main.php",
4752
},
4853
}
4954
if err := Install(t.Context(), tools, "php", binDir); err != nil {
50-
t.Fatalf("Install() error = %v", err)
55+
t.Fatal(err)
5156
}
5257
wrapperPath := filepath.Join(binDir, "gapic-generator-php")
5358
b, err := os.ReadFile(wrapperPath)
@@ -62,16 +67,74 @@ func TestInstall(t *testing.T) {
6267
}
6368

6469
func TestInstall_Error(t *testing.T) {
70+
binDir := t.TempDir()
71+
tools := []*config.ComposerTool{{Name: "", Version: "1.0.0"}}
72+
gotErr := Install(t.Context(), tools, "php", binDir)
73+
if !errors.Is(gotErr, ErrInvalidTool) {
74+
t.Fatalf("Install() error = %v, wantErr = %v", gotErr, ErrInvalidTool)
75+
}
76+
}
77+
78+
func TestInstall_NoEntrypoint(t *testing.T) {
79+
testhelper.RequireCommand(t, "composer")
80+
cache := t.TempDir()
81+
t.Setenv("LIBRARIAN_CACHE", cache)
82+
repoDir := filepath.Join(cache, "github.com/googleapis/google-cloud-php@1.0.0")
83+
if err := os.MkdirAll(repoDir, 0o755); err != nil {
84+
t.Fatal(err)
85+
}
86+
if err := os.WriteFile(filepath.Join(repoDir, "composer.json"), []byte("{}"), 0o644); err != nil {
87+
t.Fatal(err)
88+
}
89+
bin := t.TempDir()
90+
testhelper.WriteExecutable(t, filepath.Join(bin, "composer"), "#!/bin/sh\nexit 0\n")
91+
t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
6592
binDir := t.TempDir()
6693
tools := []*config.ComposerTool{
6794
{
68-
Name: "",
95+
Name: "google-cloud-php/dev",
6996
Version: "1.0.0",
97+
Repo: "github.com/googleapis/google-cloud-php",
98+
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
7099
},
71100
}
72-
gotErr := Install(t.Context(), tools, "php", binDir)
73-
if !errors.Is(gotErr, ErrInvalidTool) {
74-
t.Fatalf("Install() error = %v, wantErr = %v", gotErr, ErrInvalidTool)
101+
if err := Install(t.Context(), tools, "php", binDir); err != nil {
102+
t.Fatal(err)
103+
}
104+
wrapperPath := filepath.Join(binDir, "dev")
105+
if _, err := os.Stat(wrapperPath); err == nil || !errors.Is(err, fs.ErrNotExist) {
106+
t.Errorf("Stat() error = %v, want %v", err, fs.ErrNotExist)
107+
}
108+
}
109+
110+
func TestVerify_Entrypoint(t *testing.T) {
111+
for _, test := range []struct {
112+
name string
113+
tool *config.ComposerTool
114+
wantErr error
115+
}{
116+
{
117+
name: "absolute entrypoint",
118+
tool: &config.ComposerTool{Name: "foo", Version: "1.0.0", Repo: "bar", SHA256: "baz", Entrypoint: "/etc/passwd"},
119+
wantErr: ErrInvalidTool,
120+
},
121+
{
122+
name: "relative traversal",
123+
tool: &config.ComposerTool{Name: "foo", Version: "1.0.0", Repo: "bar", SHA256: "baz", Entrypoint: "../foo.php"},
124+
wantErr: ErrInvalidTool,
125+
},
126+
{
127+
name: "valid entrypoint",
128+
tool: &config.ComposerTool{Name: "foo", Version: "1.0.0", Repo: "bar", SHA256: "baz", Entrypoint: "src/foo.php"},
129+
wantErr: nil,
130+
},
131+
} {
132+
t.Run(test.name, func(t *testing.T) {
133+
err := verify([]*config.ComposerTool{test.tool})
134+
if !errors.Is(err, test.wantErr) {
135+
t.Errorf("verify() error = %v, wantErr %v", err, test.wantErr)
136+
}
137+
})
75138
}
76139
}
77140

@@ -124,7 +187,7 @@ func TestVerify(t *testing.T) {
124187
{Name: "gapic-generator-php", Version: "1.0.0", Repo: "github.com/googleapis/gapic-generator-php", SHA256: "somehash"},
125188
}
126189
if err := verify(tools); err != nil {
127-
t.Errorf("verify() error = %v, want nil", err)
190+
t.Error(err)
128191
}
129192
}
130193

@@ -194,15 +257,19 @@ func TestInstall_LocalPath(t *testing.T) {
194257
t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH"))
195258

196259
localDir := t.TempDir()
260+
if err := os.WriteFile(filepath.Join(localDir, "composer.json"), []byte("{}"), 0o644); err != nil {
261+
t.Fatal(err)
262+
}
197263
tools := []*config.ComposerTool{
198264
{
199-
Name: "gapic-generator-php",
200-
LocalPath: localDir,
265+
Name: "gapic-generator-php",
266+
LocalPath: localDir,
267+
Entrypoint: "src/Main.php",
201268
},
202269
}
203270
binDir := t.TempDir()
204271
if err := Install(t.Context(), tools, "php", binDir); err != nil {
205-
t.Fatalf("Install() with LocalPath error = %v", err)
272+
t.Fatal(err)
206273
}
207274
wrapperPath := filepath.Join(binDir, "gapic-generator-php")
208275
b, err := os.ReadFile(wrapperPath)
@@ -226,8 +293,9 @@ func TestInstall_LocalPath_Error(t *testing.T) {
226293
localDir := t.TempDir()
227294
tools := []*config.ComposerTool{
228295
{
229-
Name: "gapic-generator-php",
230-
LocalPath: localDir,
296+
Name: "gapic-generator-php",
297+
LocalPath: localDir,
298+
Entrypoint: "src/Main.php",
231299
},
232300
}
233301
binDir := t.TempDir()

‎tool/cmd/migrate/librarian_php.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ default:
2828
tools:
2929
composer:
3030
- name: google/gapic-generator-php
31+
entrypoint: src/Main.php
3132
version: v1.21.2
3233
repo: github.com/googleapis/gapic-generator-php
3334
sha256: 29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518

‎tool/cmd/migrate/php_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,11 @@ deep-copy-regex:
120120
Tools: &config.Tools{
121121
Composer: []*config.ComposerTool{
122122
{
123-
Name: "google/gapic-generator-php",
124-
Version: "v1.21.2",
125-
Repo: "github.com/googleapis/gapic-generator-php",
126-
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
123+
Name: "google/gapic-generator-php",
124+
Entrypoint: "src/Main.php",
125+
Version: "v1.21.2",
126+
Repo: "github.com/googleapis/gapic-generator-php",
127+
SHA256: "29635b02c6e505fe31cba2f88ae999f00d2710fe1d65cb7cad521a82e7c5a518",
127128
},
128129
},
129130
Pip: []*config.PipTool{

0 commit comments

Comments
 (0)