Skip to content

Commit c2911dc

Browse files
ramonskieramonskie
committed
refactor: consolidate processPhpIni implementations into shared config.ProcessPhpIni
Eliminated duplicate processPhpIni() implementations across supply.go and composer.go. Created shared config.ProcessPhpIni() function: - Validates PHP extensions against installed .so files and compiled modules - Builds extension directives for php.ini - Replaces #{PHP_EXTENSIONS} and #{ZEND_EXTENSIONS} placeholders - Supports additional placeholder replacements via map parameter - Accepts custom warning logger callback Updated src/php/supply/supply.go: - Reduced processPhpIni() from 92 lines to 23 lines - Delegates to config.ProcessPhpIni() with Context/Options extensions - Passes s.Log.Warning as logger callback Updated src/php/extensions/composer/composer.go: - Reduced processPhpIni() from 112 lines to 63 lines - Delegates core logic to config.ProcessPhpIni() - Passes path placeholders (@{HOME}, @{TMPDIR}, #{LIBDIR}) as replacements - Keeps composer-specific extension_dir fixup logic after shared call Result: ~160 lines of duplicate code eliminated Total reduction so far: ~538 lines across 4 commits All 144 unit tests pass
1 parent b363a0a commit c2911dc

3 files changed

Lines changed: 128 additions & 158 deletions

File tree

‎src/php/config/config.go‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ import (
77
"io/fs"
88
"os"
99
"path/filepath"
10+
"strings"
11+
12+
"github.com/cloudfoundry/php-buildpack/src/php/util"
1013
)
1114

1215
//go:embed defaults
@@ -195,3 +198,88 @@ func CopyWithSubstitution(src io.Reader, dest io.Writer, vars map[string]string)
195198
_, err = dest.Write([]byte(content))
196199
return err
197200
}
201+
202+
// ProcessPhpIni processes php.ini file by validating extensions and replacing placeholders
203+
func ProcessPhpIni(
204+
phpIniPath string,
205+
phpInstallDir string,
206+
phpExtensions []string,
207+
zendExtensions []string,
208+
additionalReplacements map[string]string,
209+
logWarning func(string, ...interface{}),
210+
) error {
211+
content, err := os.ReadFile(phpIniPath)
212+
if err != nil {
213+
return fmt.Errorf("failed to read php.ini: %w", err)
214+
}
215+
216+
phpIniContent := string(content)
217+
218+
skipExtensions := map[string]bool{
219+
"cli": true,
220+
"pear": true,
221+
"cgi": true,
222+
}
223+
224+
phpExtDir := ""
225+
phpLibDir := filepath.Join(phpInstallDir, "lib", "php", "extensions")
226+
if entries, err := os.ReadDir(phpLibDir); err == nil {
227+
for _, entry := range entries {
228+
if entry.IsDir() && strings.HasPrefix(entry.Name(), "no-debug-non-zts-") {
229+
phpExtDir = filepath.Join(phpLibDir, entry.Name())
230+
break
231+
}
232+
}
233+
}
234+
235+
phpBinary := filepath.Join(phpInstallDir, "bin", "php")
236+
phpLib := filepath.Join(phpInstallDir, "lib")
237+
compiledModules, err := util.GetCompiledModules(phpBinary, phpLib)
238+
if err != nil {
239+
if logWarning != nil {
240+
logWarning("Failed to get compiled PHP modules: %v", err)
241+
}
242+
compiledModules = make(map[string]bool)
243+
}
244+
245+
var extensionLines []string
246+
for _, ext := range phpExtensions {
247+
if skipExtensions[ext] {
248+
continue
249+
}
250+
251+
if phpExtDir != "" {
252+
extFile := filepath.Join(phpExtDir, ext+".so")
253+
exists := false
254+
if info, err := os.Stat(extFile); err == nil && !info.IsDir() {
255+
exists = true
256+
}
257+
258+
if exists {
259+
extensionLines = append(extensionLines, fmt.Sprintf("extension=%s.so", ext))
260+
} else if !compiledModules[strings.ToLower(ext)] {
261+
fmt.Printf("The extension '%s' is not provided by this buildpack.\n", ext)
262+
}
263+
}
264+
}
265+
extensionsString := strings.Join(extensionLines, "\n")
266+
267+
var zendExtensionLines []string
268+
for _, ext := range zendExtensions {
269+
zendExtensionLines = append(zendExtensionLines, fmt.Sprintf("zend_extension=\"%s.so\"", ext))
270+
}
271+
zendExtensionsString := strings.Join(zendExtensionLines, "\n")
272+
273+
phpIniContent = strings.ReplaceAll(phpIniContent, "#{PHP_EXTENSIONS}", extensionsString)
274+
phpIniContent = strings.ReplaceAll(phpIniContent, "#{ZEND_EXTENSIONS}", zendExtensionsString)
275+
276+
for placeholder, value := range additionalReplacements {
277+
phpIniContent = strings.ReplaceAll(phpIniContent, placeholder, value)
278+
}
279+
280+
if err := os.WriteFile(phpIniPath, []byte(phpIniContent), 0644); err != nil {
281+
return fmt.Errorf("failed to write php.ini: %w", err)
282+
}
283+
284+
return nil
285+
}

‎src/php/extensions/composer/composer.go‎

Lines changed: 30 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -919,28 +919,33 @@ func (e *ComposerExtension) setupPHPConfig(ctx *extensions.Context) error {
919919
return nil
920920
}
921921

922-
// processPhpIni processes php.ini to replace extension placeholders with actual extension directives
923922
func (e *ComposerExtension) processPhpIni(ctx *extensions.Context, phpIniPath string) error {
924-
// Read the php.ini file
925-
content, err := os.ReadFile(phpIniPath)
926-
if err != nil {
927-
return fmt.Errorf("failed to read php.ini: %w", err)
928-
}
929-
930-
phpIniContent := string(content)
931-
932-
// Get PHP extensions from context
933923
phpExtensions := ctx.GetStringSlice("PHP_EXTENSIONS")
934924
zendExtensions := ctx.GetStringSlice("ZEND_EXTENSIONS")
935925

936-
// Skip certain extensions that should not be in php.ini (they're CLI-only or built-in)
937-
skipExtensions := map[string]bool{
938-
"cli": true,
939-
"pear": true,
940-
"cgi": true,
926+
phpInstallDir := filepath.Join(e.buildDir, "php")
927+
928+
additionalReplacements := map[string]string{
929+
"@{HOME}": e.buildDir,
930+
"@{TMPDIR}": e.tmpDir,
931+
"#{LIBDIR}": e.libDir,
932+
}
933+
934+
logWarning := func(format string, args ...interface{}) {
935+
fmt.Printf(" WARNING: "+format+"\n", args...)
936+
}
937+
938+
if err := config.ProcessPhpIni(
939+
phpIniPath,
940+
phpInstallDir,
941+
phpExtensions,
942+
zendExtensions,
943+
additionalReplacements,
944+
logWarning,
945+
); err != nil {
946+
return err
941947
}
942948

943-
// Find PHP extensions directory to validate requested extensions
944949
phpExtDir := ""
945950
phpLibDir := filepath.Join(e.buildDir, "php", "lib", "php", "extensions")
946951
if entries, err := os.ReadDir(phpLibDir); err == nil {
@@ -952,83 +957,29 @@ func (e *ComposerExtension) processPhpIni(ctx *extensions.Context, phpIniPath st
952957
}
953958
}
954959

955-
// Get list of built-in PHP modules (extensions compiled into PHP core)
956-
phpBinary := filepath.Join(e.buildDir, "php", "bin", "php")
957-
phpLib := filepath.Join(e.buildDir, "php", "lib")
958-
compiledModules, err := util.GetCompiledModules(phpBinary, phpLib)
959-
if err != nil {
960-
fmt.Printf(" WARNING: Failed to get compiled PHP modules: %v\n", err)
961-
compiledModules = make(map[string]bool) // Continue without built-in module list
962-
}
963-
964-
// Build extension directives and validate extensions
965-
var extensionLines []string
966-
for _, ext := range phpExtensions {
967-
if skipExtensions[ext] {
968-
continue
969-
}
970-
971-
// Check if extension .so file exists
972-
if phpExtDir != "" {
973-
extFile := filepath.Join(phpExtDir, ext+".so")
974-
exists := false
975-
if info, err := os.Stat(extFile); err == nil && !info.IsDir() {
976-
exists = true
977-
}
978-
979-
if exists {
980-
// Extension has .so file, add to php.ini
981-
extensionLines = append(extensionLines, fmt.Sprintf("extension=%s.so", ext))
982-
} else if !compiledModules[strings.ToLower(ext)] {
983-
// Extension doesn't have .so file AND is not built-in -> warn
984-
fmt.Printf("The extension '%s' is not provided by this buildpack.\n", ext)
985-
}
986-
// If it's built-in (no .so but in compiled modules), silently skip - it's already available
960+
if phpExtDir != "" {
961+
content, err := os.ReadFile(phpIniPath)
962+
if err != nil {
963+
return fmt.Errorf("failed to read php.ini: %w", err)
987964
}
988-
}
989-
extensionsString := strings.Join(extensionLines, "\n")
990965

991-
// Build zend extension directives
992-
var zendExtensionLines []string
993-
for _, ext := range zendExtensions {
994-
zendExtensionLines = append(zendExtensionLines, fmt.Sprintf("zend_extension=\"%s.so\"", ext))
995-
}
996-
zendExtensionsString := strings.Join(zendExtensionLines, "\n")
997-
998-
// Replace placeholders
999-
phpIniContent = strings.ReplaceAll(phpIniContent, "#{PHP_EXTENSIONS}", extensionsString)
1000-
phpIniContent = strings.ReplaceAll(phpIniContent, "#{ZEND_EXTENSIONS}", zendExtensionsString)
1001-
1002-
// Replace path placeholders (@{HOME}, @{TMPDIR}, #{LIBDIR})
1003-
// @{HOME} should be the build directory, not build_dir/php
1004-
// The template already has paths like @{HOME}/php/lib/...
1005-
phpIniContent = strings.ReplaceAll(phpIniContent, "@{HOME}", e.buildDir)
1006-
phpIniContent = strings.ReplaceAll(phpIniContent, "@{TMPDIR}", e.tmpDir)
1007-
phpIniContent = strings.ReplaceAll(phpIniContent, "#{LIBDIR}", e.libDir)
1008-
1009-
// Fix extension_dir to use the actual discovered path
1010-
// During Composer phase, PHP is installed in BUILD_DIR/php
1011-
// The phpExtDir variable already contains the correct full path
1012-
if phpExtDir != "" {
1013-
// Find and replace the extension_dir line with the actual path
966+
phpIniContent := string(content)
1014967
lines := strings.Split(phpIniContent, "\n")
1015968
for i, line := range lines {
1016969
trimmed := strings.TrimSpace(line)
1017970
if strings.HasPrefix(trimmed, "extension_dir") && !strings.HasPrefix(trimmed, ";") {
1018-
// This is the active extension_dir line - replace it with actual path
1019971
lines[i] = fmt.Sprintf("extension_dir = \"%s\"", phpExtDir)
1020972
break
1021973
}
1022974
}
1023975
phpIniContent = strings.Join(lines, "\n")
1024-
}
1025976

1026-
// Write back to php.ini
1027-
if err := os.WriteFile(phpIniPath, []byte(phpIniContent), 0644); err != nil {
1028-
return fmt.Errorf("failed to write php.ini: %w", err)
977+
if err := os.WriteFile(phpIniPath, []byte(phpIniContent), 0644); err != nil {
978+
return fmt.Errorf("failed to write php.ini: %w", err)
979+
}
1029980
}
1030981

1031-
fmt.Printf(" Configured PHP with %d extensions\n", len(extensionLines))
982+
fmt.Printf(" Configured PHP with extensions\n")
1032983
return nil
1033984
}
1034985

‎src/php/supply/supply.go‎

Lines changed: 10 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -436,17 +436,7 @@ func (s *Supplier) InstallPHP() error {
436436
return nil
437437
}
438438

439-
// processPhpIni processes php.ini to replace extension placeholders with actual extension directives
440439
func (s *Supplier) processPhpIni(phpIniPath string) error {
441-
// Read the php.ini file
442-
content, err := os.ReadFile(phpIniPath)
443-
if err != nil {
444-
return fmt.Errorf("failed to read php.ini: %w", err)
445-
}
446-
447-
phpIniContent := string(content)
448-
449-
// Get PHP extensions from context if available, otherwise from Options
450440
var phpExtensions, zendExtensions []string
451441
if s.Context != nil {
452442
phpExtensions = s.Context.GetStringSlice("PHP_EXTENSIONS")
@@ -456,79 +446,20 @@ func (s *Supplier) processPhpIni(phpIniPath string) error {
456446
zendExtensions = s.Options.ZendExtensions
457447
}
458448

459-
// Skip certain extensions that should not be in php.ini (they're CLI-only or built-in)
460-
skipExtensions := map[string]bool{
461-
"cli": true,
462-
"pear": true,
463-
"cgi": true,
464-
}
465-
466-
// Find PHP extensions directory to validate requested extensions
467449
phpInstallDir := filepath.Join(s.Stager.DepDir(), "php")
468-
phpExtDir := ""
469-
470-
// Look for extensions directory: php/lib/php/extensions/no-debug-non-zts-*/
471-
phpLibDir := filepath.Join(phpInstallDir, "lib", "php", "extensions")
472-
if entries, err := os.ReadDir(phpLibDir); err == nil {
473-
for _, entry := range entries {
474-
if entry.IsDir() && strings.HasPrefix(entry.Name(), "no-debug-non-zts-") {
475-
phpExtDir = filepath.Join(phpLibDir, entry.Name())
476-
break
477-
}
478-
}
479-
}
480-
481-
// Get list of built-in PHP modules (extensions compiled into PHP core)
482-
phpBinary := filepath.Join(phpInstallDir, "bin", "php")
483-
phpLib := filepath.Join(phpInstallDir, "lib")
484-
compiledModules, err := util.GetCompiledModules(phpBinary, phpLib)
485-
if err != nil {
486-
s.Log.Warning("Failed to get compiled PHP modules: %v", err)
487-
compiledModules = make(map[string]bool) // Continue without built-in module list
488-
}
489450

490-
// Build extension directives and validate extensions
491-
var extensionLines []string
492-
for _, ext := range phpExtensions {
493-
if skipExtensions[ext] {
494-
continue
495-
}
496-
497-
// Check if extension .so file exists
498-
if phpExtDir != "" {
499-
extFile := filepath.Join(phpExtDir, ext+".so")
500-
if exists, _ := libbuildpack.FileExists(extFile); exists {
501-
// Extension has .so file, add to php.ini
502-
extensionLines = append(extensionLines, fmt.Sprintf("extension=%s.so", ext))
503-
} else if !compiledModules[strings.ToLower(ext)] {
504-
// Extension doesn't have .so file AND is not built-in -> warn
505-
fmt.Printf("The extension '%s' is not provided by this buildpack.\n", ext)
506-
}
507-
// If it's built-in (no .so but in compiled modules), silently skip - it's already available
508-
}
451+
logWarning := func(format string, args ...interface{}) {
452+
s.Log.Warning(format, args...)
509453
}
510-
extensionsString := strings.Join(extensionLines, "\n")
511454

512-
// Build zend extension directives
513-
var zendExtensionLines []string
514-
for _, ext := range zendExtensions {
515-
zendExtensionLines = append(zendExtensionLines, fmt.Sprintf("zend_extension=\"%s.so\"", ext))
516-
}
517-
zendExtensionsString := strings.Join(zendExtensionLines, "\n")
518-
519-
// Replace build-time-only placeholders
520-
// Note: Runtime placeholders like @{HOME}, @{TMPDIR}, #{WEBDIR}, #{LIBDIR} are left as-is
521-
// and will be replaced by the rewrite tool at runtime (in start script)
522-
phpIniContent = strings.ReplaceAll(phpIniContent, "#{PHP_EXTENSIONS}", extensionsString)
523-
phpIniContent = strings.ReplaceAll(phpIniContent, "#{ZEND_EXTENSIONS}", zendExtensionsString)
524-
525-
// Write back to php.ini
526-
if err := os.WriteFile(phpIniPath, []byte(phpIniContent), 0644); err != nil {
527-
return fmt.Errorf("failed to write php.ini: %w", err)
528-
}
529-
530-
s.Log.Debug("Processed php.ini with %d extensions and %d zend extensions", len(extensionLines), len(zendExtensionLines))
531-
return nil
455+
return config.ProcessPhpIni(
456+
phpIniPath,
457+
phpInstallDir,
458+
phpExtensions,
459+
zendExtensions,
460+
nil,
461+
logWarning,
462+
)
532463
}
533464

534465
// processPhpFpmConf processes php-fpm.conf to set the include directive for fpm.d configs

0 commit comments

Comments
 (0)