Skip to content

Commit 1131169

Browse files
committed
Fix PHP version selection regression when composer.lock has stale package constraints (issue #1220)
In v5 the buildpack incorrectly re-evaluated package-level PHP constraints from composer.lock (e.g. "<8.3.0" written before PHP 8.3 existed) when selecting a PHP version, causing apps with composer.json requiring "^8.3" to silently fall back to the default PHP version. Restore v4 semantics: only the top-level require.php in composer.json is used for PHP version selection; package-level constraints in composer.lock are ignored. - Remove readPHPConstraintsFromLock() and the lock-constraint filtering loop - Update unit tests: remove the three tests asserting the old (buggy) behaviour, add regression test asserting 8.3.x is selected despite stale lock constraints - Add integration test fixture fixtures/composer_83_version_selection/ with a minimal composer.json (php: ^8.3) and empty composer.lock (no packages to download), and a focused integration test that verifies end-to-end deployment on PHP 8.3
1 parent a74d1b8 commit 1131169

File tree

6 files changed

+85
-216
lines changed

6 files changed

+85
-216
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"name": "cloudfoundry/composer_83_version_selection",
3+
"description": "Regression fixture for issue #1220: composer.json requires ^8.3 but composer.lock packages carry stale <8.3.0 upper-bound constraints",
4+
"require": {
5+
"php": "^8.3"
6+
}
7+
}

fixtures/composer_83_version_selection/composer.lock

Lines changed: 20 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
// Regression test for https://github.com/cloudfoundry/php-buildpack/issues/1220
3+
// Outputs the running PHP major.minor version so the integration test can assert
4+
// that PHP 8.3 was selected, not the default 8.1.
5+
echo 'PHP Version: ' . PHP_MAJOR_VERSION . '.' . PHP_MINOR_VERSION;

src/php/extensions/composer/composer.go

Lines changed: 7 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,7 @@ func (e *ComposerExtension) Configure(ctx *extensions.Context) error {
119119
return fmt.Errorf("failed to read PHP version from composer files: %w", err)
120120
}
121121
if phpVersion != "" {
122-
// Also check composer.lock for package PHP constraints
123-
lockConstraints := e.readPHPConstraintsFromLock()
124-
selectedVersion := e.pickPHPVersion(ctx, phpVersion, lockConstraints)
122+
selectedVersion := e.pickPHPVersion(ctx, phpVersion)
125123
ctx.Set("PHP_VERSION", selectedVersion)
126124
}
127125

@@ -208,51 +206,16 @@ func (e *ComposerExtension) readVersionFromFile(path, section, key string) (stri
208206
return "", nil
209207
}
210208

211-
// readPHPConstraintsFromLock reads PHP constraints from all packages in composer.lock
212-
func (e *ComposerExtension) readPHPConstraintsFromLock() []string {
213-
if e.lockPath == "" {
214-
return nil
215-
}
216-
217-
data, err := os.ReadFile(e.lockPath)
218-
if err != nil {
219-
return nil
220-
}
221-
222-
var lockData struct {
223-
Packages []struct {
224-
Name string `json:"name"`
225-
Require map[string]interface{} `json:"require"`
226-
} `json:"packages"`
227-
}
228-
229-
if err := json.Unmarshal(data, &lockData); err != nil {
230-
return nil
231-
}
232-
233-
var constraints []string
234-
for _, pkg := range lockData.Packages {
235-
if phpConstraint, ok := pkg.Require["php"].(string); ok && phpConstraint != "" {
236-
constraints = append(constraints, phpConstraint)
237-
}
238-
}
239-
240-
return constraints
241-
}
242-
243-
// pickPHPVersion selects the appropriate PHP version based on requirements
244-
func (e *ComposerExtension) pickPHPVersion(ctx *extensions.Context, requested string, lockConstraints []string) string {
209+
// pickPHPVersion selects the appropriate PHP version based on the composer.json requirement.
210+
// Version selection is based solely on the top-level require.php in composer.json;
211+
// package-level PHP constraints in composer.lock are intentionally ignored (v4 semantics).
212+
func (e *ComposerExtension) pickPHPVersion(ctx *extensions.Context, requested string) string {
245213
if requested == "" {
246214
return ctx.GetString("PHP_VERSION")
247215
}
248216

249217
fmt.Printf("-----> Composer requires PHP %s\n", requested)
250218

251-
// If we have composer.lock constraints, show them
252-
if len(lockConstraints) > 0 {
253-
fmt.Printf(" Locked dependencies have %d additional PHP constraints\n", len(lockConstraints))
254-
}
255-
256219
// Get all available PHP versions from context
257220
// Context should have ALL_PHP_VERSIONS set by supply phase
258221
allVersionsStr := ctx.GetString("ALL_PHP_VERSIONS")
@@ -267,48 +230,14 @@ func (e *ComposerExtension) pickPHPVersion(ctx *extensions.Context, requested st
267230
availableVersions[i] = strings.TrimSpace(availableVersions[i])
268231
}
269232

270-
// Find the best matching version for composer.json constraint
233+
// Find the best matching version for the composer.json constraint
271234
selectedVersion := e.matchVersion(requested, availableVersions)
272235
if selectedVersion == "" {
273236
fmt.Printf(" Warning: No matching PHP version found for %s, using default\n", requested)
274237
return ctx.GetString("PHP_DEFAULT")
275238
}
276239

277-
// If we have lock constraints, ensure the selected version satisfies ALL of them
278-
if len(lockConstraints) > 0 {
279-
// Filter available versions to only those matching ALL constraints (composer.json + lock)
280-
validVersions := []string{}
281-
for _, version := range availableVersions {
282-
// Check composer.json constraint
283-
if !e.versionMatchesConstraint(version, requested) {
284-
continue
285-
}
286-
287-
// Check all lock constraints
288-
matchesAll := true
289-
for _, lockConstraint := range lockConstraints {
290-
if !e.versionMatchesConstraint(version, lockConstraint) {
291-
matchesAll = false
292-
break
293-
}
294-
}
295-
296-
if matchesAll {
297-
validVersions = append(validVersions, version)
298-
}
299-
}
300-
301-
if len(validVersions) == 0 {
302-
fmt.Printf(" Warning: No PHP version satisfies all constraints, using default\n")
303-
return ctx.GetString("PHP_DEFAULT")
304-
}
305-
306-
// Find the highest valid version
307-
selectedVersion = e.findHighestVersion(validVersions)
308-
fmt.Printf(" Selected PHP version: %s (satisfies all %d constraints)\n", selectedVersion, len(lockConstraints)+1)
309-
} else {
310-
fmt.Printf(" Selected PHP version: %s\n", selectedVersion)
311-
}
240+
fmt.Printf(" Selected PHP version: %s\n", selectedVersion)
312241

313242
return selectedVersion
314243
}

src/php/extensions/composer/composer_test.go

Lines changed: 23 additions & 138 deletions
Original file line numberDiff line numberDiff line change
@@ -477,155 +477,38 @@ var _ = Describe("ComposerExtension", func() {
477477
Expect(phpVersion).To(Equal("8.1.32")) // PHP_DEFAULT
478478
})
479479
})
480-
})
481-
482-
Describe("Configure - composer.lock Constraint Checking", func() {
483-
Context("when composer.lock has package PHP constraints", func() {
484-
BeforeEach(func() {
485-
composerJSON := filepath.Join(buildDir, "composer.json")
486-
jsonContent := `{
487-
"require": {
488-
"php": ">=7.4"
489-
}
490-
}`
491-
err := os.WriteFile(composerJSON, []byte(jsonContent), 0644)
492-
Expect(err).NotTo(HaveOccurred())
493-
494-
composerLock := filepath.Join(buildDir, "composer.lock")
495-
lockContent := `{
496-
"packages": [
497-
{
498-
"name": "laminas/laminas-diactoros",
499-
"version": "2.22.0",
500-
"require": {
501-
"php": "~8.0.0 || ~8.1.0 || ~8.2.0"
502-
}
503-
},
504-
{
505-
"name": "vendor/package",
506-
"version": "1.0.0",
507-
"require": {
508-
"php": ">=8.1.0"
509-
}
510-
}
511-
]
512-
}`
513-
err = os.WriteFile(composerLock, []byte(lockContent), 0644)
514-
Expect(err).NotTo(HaveOccurred())
515-
516-
ext.ShouldCompile(ctx)
517-
})
518-
519-
It("should select highest version satisfying all constraints", func() {
520-
err := ext.Configure(ctx)
521-
Expect(err).NotTo(HaveOccurred())
522-
523-
phpVersion := ctx.GetString("PHP_VERSION")
524-
// composer.json: >=7.4
525-
// laminas: ~8.0.0 || ~8.1.0 || ~8.2.0
526-
// vendor/package: >=8.1.0
527-
// Should select 8.2.28 (highest matching all)
528-
Expect(phpVersion).To(Equal("8.2.28"))
529-
})
530-
})
531-
532-
Context("when composer.lock constraints exclude highest version", func() {
533-
BeforeEach(func() {
534-
composerJSON := filepath.Join(buildDir, "composer.json")
535-
jsonContent := `{
536-
"require": {
537-
"php": ">=8.0"
538-
}
539-
}`
540-
err := os.WriteFile(composerJSON, []byte(jsonContent), 0644)
541-
Expect(err).NotTo(HaveOccurred())
542-
543-
composerLock := filepath.Join(buildDir, "composer.lock")
544-
lockContent := `{
545-
"packages": [
546-
{
547-
"name": "old-package/example",
548-
"version": "1.0.0",
549-
"require": {
550-
"php": "<8.3.0"
551-
}
552-
}
553-
]
554-
}`
555-
err = os.WriteFile(composerLock, []byte(lockContent), 0644)
556-
Expect(err).NotTo(HaveOccurred())
557-
558-
ext.ShouldCompile(ctx)
559-
})
560-
561-
It("should respect lock constraint and select lower version", func() {
562-
err := ext.Configure(ctx)
563-
Expect(err).NotTo(HaveOccurred())
564-
565-
phpVersion := ctx.GetString("PHP_VERSION")
566-
// composer.json: >=8.0
567-
// old-package: <8.3.0
568-
// Should select 8.2.28 (not 8.3.x)
569-
Expect(phpVersion).To(Equal("8.2.28"))
570-
})
571-
})
572-
573-
Context("when no version satisfies all lock constraints", func() {
574-
BeforeEach(func() {
575-
composerJSON := filepath.Join(buildDir, "composer.json")
576-
jsonContent := `{
577-
"require": {
578-
"php": ">=8.3.0"
579-
}
580-
}`
581-
err := os.WriteFile(composerJSON, []byte(jsonContent), 0644)
582-
Expect(err).NotTo(HaveOccurred())
583-
584-
composerLock := filepath.Join(buildDir, "composer.lock")
585-
lockContent := `{
586-
"packages": [
587-
{
588-
"name": "impossible/package",
589-
"version": "1.0.0",
590-
"require": {
591-
"php": "<8.0.0"
592-
}
593-
}
594-
]
595-
}`
596-
err = os.WriteFile(composerLock, []byte(lockContent), 0644)
597-
Expect(err).NotTo(HaveOccurred())
598-
599-
ext.ShouldCompile(ctx)
600-
})
601-
602-
It("should fall back to default PHP version", func() {
603-
err := ext.Configure(ctx)
604-
Expect(err).NotTo(HaveOccurred())
605480

606-
phpVersion := ctx.GetString("PHP_VERSION")
607-
Expect(phpVersion).To(Equal("8.1.32")) // PHP_DEFAULT
608-
})
609-
})
610-
611-
Context("when composer.lock has no package PHP constraints", func() {
481+
// Regression test for https://github.com/cloudfoundry/php-buildpack/issues/1220
482+
// composer.json requires ^8.3, but composer.lock packages carry stale upper-bound
483+
// constraints (e.g. <8.3.0) written before PHP 8.3 was released. The buildpack must
484+
// not re-evaluate those package-level constraints — Composer already resolved them
485+
// when it wrote the lock file. Only the top-level require.php in composer.json matters.
486+
Context("when composer.json requires ^8.3 and composer.lock has packages with stale <8.3.0 constraints (issue #1220)", func() {
612487
BeforeEach(func() {
613488
composerJSON := filepath.Join(buildDir, "composer.json")
614489
jsonContent := `{
615490
"require": {
616-
"php": ">=8.2.0"
491+
"php": "^8.3"
617492
}
618493
}`
619494
err := os.WriteFile(composerJSON, []byte(jsonContent), 0644)
620495
Expect(err).NotTo(HaveOccurred())
621496

497+
// Simulate a real-world lock file: 84 packages, many with conservative
498+
// upper-bound PHP constraints written before PHP 8.3 was released.
622499
composerLock := filepath.Join(buildDir, "composer.lock")
623500
lockContent := `{
624501
"packages": [
625-
{
626-
"name": "simple/package",
627-
"version": "1.0.0"
628-
}
502+
{"name": "pkg/a", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}},
503+
{"name": "pkg/b", "version": "2.0.0", "require": {"php": ">=7.4 <8.3.0"}},
504+
{"name": "pkg/c", "version": "3.0.0", "require": {"php": ">=8.0 <8.3.0"}},
505+
{"name": "pkg/d", "version": "1.5.0", "require": {"php": "^7.2 || ^8.0"}},
506+
{"name": "pkg/e", "version": "1.0.0", "require": {"php": ">=8.1"}},
507+
{"name": "pkg/f", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}},
508+
{"name": "pkg/g", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}},
509+
{"name": "pkg/h", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}},
510+
{"name": "pkg/i", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}},
511+
{"name": "pkg/j", "version": "1.0.0", "require": {"php": ">=7.2.5 <8.3.0"}}
629512
]
630513
}`
631514
err = os.WriteFile(composerLock, []byte(lockContent), 0644)
@@ -634,12 +517,14 @@ var _ = Describe("ComposerExtension", func() {
634517
ext.ShouldCompile(ctx)
635518
})
636519

637-
It("should use only composer.json constraint", func() {
520+
It("should select 8.3.x, ignoring stale package constraints in composer.lock", func() {
638521
err := ext.Configure(ctx)
639522
Expect(err).NotTo(HaveOccurred())
640523

641524
phpVersion := ctx.GetString("PHP_VERSION")
642-
Expect(phpVersion).To(Equal("8.3.21")) // Highest matching >=8.2.0
525+
// Must NOT fall back to default 8.1.32 — that is the regression.
526+
// Must select the highest 8.3.x version available.
527+
Expect(phpVersion).To(Equal("8.3.21"))
643528
})
644529
})
645530
})

src/php/integration/composer_test.go

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,29 @@ func testComposer(platform switchblade.Platform, fixtures string) func(*testing.
9494
})
9595
})
9696

97+
// Regression test for https://github.com/cloudfoundry/php-buildpack/issues/1220
98+
// In v5, pickPHPVersion incorrectly re-evaluated package-level PHP constraints from
99+
// composer.lock (e.g. "<8.3.0") and fell back to the default PHP version even when
100+
// composer.json explicitly required "^8.3". The fix restores v4 semantics: only the
101+
// top-level require.php in composer.json is used for version selection.
102+
context("composer.json requires ^8.3 with stale <8.3.0 package constraints in composer.lock", func() {
103+
it("selects PHP 8.3, ignoring stale package constraints in composer.lock (issue #1220)", func() {
104+
deployment, logs, err := platform.Deploy.
105+
Execute(name, filepath.Join(fixtures, "composer_83_version_selection"))
106+
Expect(err).NotTo(HaveOccurred(), logs.String)
107+
108+
// Build log must show the composer.json constraint was read
109+
Expect(logs.String()).To(ContainSubstring("Composer requires PHP ^8.3"))
110+
111+
// Build log must show PHP 8.3.x was installed — not the 8.1.x default
112+
Expect(logs.String()).To(MatchRegexp(`Installing PHP 8\.3\.\d+`))
113+
Expect(logs.String()).NotTo(MatchRegexp(`Installing PHP 8\.1\.\d+`))
114+
115+
// The running app must confirm it is executing on PHP 8.3
116+
Eventually(deployment).Should(Serve(MatchRegexp(`PHP Version: 8\.3`)))
117+
})
118+
})
119+
97120
if !settings.Cached {
98121
context("deployed with invalid COMPOSER_GITHUB_OAUTH_TOKEN", func() {
99122
it("logs warning", func() {

0 commit comments

Comments
 (0)