Skip to content

Commit b363a0a

Browse files
committed
refactor: consolidate copyFile implementations into shared util.CopyFile
Eliminated 3 duplicate copyFile() implementations across the codebase: - composer.go (io.Copy, no permission handling) - supply.go (io.Copy + explicit os.Chmod) - finalize.go (ReadFile + WriteFile with mode) Created unified util.CopyFile() that: - Preserves file permissions by copying source file mode - Uses os.ReadFile/WriteFile pattern (simplest and most robust) - Replaces all 3 local implementations Files modified: - src/php/util/util.go: Added CopyFile() function - src/php/extensions/composer/composer.go: Removed copyFile(), updated call site - src/php/supply/supply.go: Removed copyFile(), updated call site - src/php/finalize/finalize.go: Removed copyFile(), updated call site, added util import Result: ~50 lines of duplicate code eliminated All tests passing: 144 specs across 8 test suites
1 parent 0a81ffa commit b363a0a

4 files changed

Lines changed: 19 additions & 65 deletions

File tree

src/php/extensions/composer/composer.go

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -912,7 +912,7 @@ func (e *ComposerExtension) setupPHPConfig(ctx *extensions.Context) error {
912912
// Copy processed php.ini to TMPDIR for Composer to use
913913
// This matches the Python buildpack behavior where PHPRC points to TMPDIR
914914
tmpPhpIniPath := filepath.Join(e.tmpDir, "php.ini")
915-
if err := e.copyFile(phpIniPath, tmpPhpIniPath); err != nil {
915+
if err := util.CopyFile(phpIniPath, tmpPhpIniPath); err != nil {
916916
return fmt.Errorf("failed to copy php.ini to TMPDIR: %w", err)
917917
}
918918

@@ -1174,24 +1174,6 @@ func (e *ComposerExtension) ServiceEnvironment(ctx *extensions.Context) (map[str
11741174
return nil, nil
11751175
}
11761176

1177-
// copyFile copies a file from src to dst
1178-
func (e *ComposerExtension) copyFile(src, dst string) error {
1179-
sourceFile, err := os.Open(src)
1180-
if err != nil {
1181-
return err
1182-
}
1183-
defer sourceFile.Close()
1184-
1185-
destFile, err := os.Create(dst)
1186-
if err != nil {
1187-
return err
1188-
}
1189-
defer destFile.Close()
1190-
1191-
_, err = io.Copy(destFile, sourceFile)
1192-
return err
1193-
}
1194-
11951177
func (e *ComposerExtension) loadUserExtensions(ctx *extensions.Context) error {
11961178
return extensions.LoadUserExtensions(ctx, e.buildDir)
11971179
}

src/php/finalize/finalize.go

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"github.com/cloudfoundry/libbuildpack"
1010
"github.com/cloudfoundry/php-buildpack/src/php/extensions"
1111
"github.com/cloudfoundry/php-buildpack/src/php/options"
12+
"github.com/cloudfoundry/php-buildpack/src/php/util"
1213
)
1314

1415
// Stager interface abstracts buildpack staging operations
@@ -231,7 +232,7 @@ func (f *Finalizer) CreateStartScript() error {
231232
}
232233
rewriteSrc := filepath.Join(bpDir, "bin", "rewrite")
233234
rewriteDst := filepath.Join(bpBinDir, "rewrite")
234-
if err := copyFile(rewriteSrc, rewriteDst); err != nil {
235+
if err := util.CopyFile(rewriteSrc, rewriteDst); err != nil {
235236
return fmt.Errorf("could not copy rewrite binary: %v", err)
236237
}
237238
f.Log.Debug("Copied rewrite binary to .bp/bin")
@@ -351,24 +352,6 @@ func (f *Finalizer) CreatePHPRuntimeDirectories() error {
351352
return nil
352353
}
353354

354-
// copyFile copies a file from src to dst with the same permissions
355-
func copyFile(src, dst string) error {
356-
// Read source file
357-
data, err := os.ReadFile(src)
358-
if err != nil {
359-
return err
360-
}
361-
362-
// Get source file info for permissions
363-
srcInfo, err := os.Stat(src)
364-
if err != nil {
365-
return err
366-
}
367-
368-
// Write destination file with same permissions
369-
return os.WriteFile(dst, data, srcInfo.Mode())
370-
}
371-
372355
// generateHTTPDStartScript generates a start script for Apache HTTPD with PHP-FPM
373356
func (f *Finalizer) generateHTTPDStartScript(depsIdx string, opts *options.Options) string {
374357
// Load options to get WEBDIR and other config values

src/php/supply/supply.go

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -775,36 +775,10 @@ func (s *Supplier) copyUserConfigs(srcDir, destDir string) error {
775775

776776
// If it's a file, copy it
777777
s.Log.Debug("Copying user config: %s -> %s", path, destPath)
778-
return s.copyFile(path, destPath)
778+
return util.CopyFile(path, destPath)
779779
})
780780
}
781781

782-
// copyFile copies a single file from src to dest
783-
func (s *Supplier) copyFile(src, dest string) error {
784-
sourceFile, err := os.Open(src)
785-
if err != nil {
786-
return err
787-
}
788-
defer sourceFile.Close()
789-
790-
destFile, err := os.Create(dest)
791-
if err != nil {
792-
return err
793-
}
794-
defer destFile.Close()
795-
796-
if _, err := io.Copy(destFile, sourceFile); err != nil {
797-
return err
798-
}
799-
800-
// Copy file permissions
801-
sourceInfo, err := os.Stat(src)
802-
if err != nil {
803-
return err
804-
}
805-
return os.Chmod(dest, sourceInfo.Mode())
806-
}
807-
808782
// ProcessPhpFpmConfForTesting exposes processPhpFpmConf for testing purposes
809783
func (s *Supplier) ProcessPhpFpmConfForTesting(phpFpmConfPath, phpEtcDir string) error {
810784
return s.processPhpFpmConf(phpFpmConfPath, phpEtcDir)

src/php/util/util.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,3 +85,18 @@ func CompareVersions(v1, v2 string) int {
8585

8686
return 0
8787
}
88+
89+
// CopyFile copies a file from src to dst preserving file permissions
90+
func CopyFile(src, dst string) error {
91+
data, err := os.ReadFile(src)
92+
if err != nil {
93+
return err
94+
}
95+
96+
srcInfo, err := os.Stat(src)
97+
if err != nil {
98+
return err
99+
}
100+
101+
return os.WriteFile(dst, data, srcInfo.Mode())
102+
}

0 commit comments

Comments
 (0)