Skip to content

Commit 2147c05

Browse files
committed
feat: add support for variant selection
As a maintainer I need on a regular basis to pick specific server type to reproduce issues. But patching source code to do so is: a. cumbersome b. error prone And sometimes this can be useful for users as well (imagine you have a 8.4.5 installed with CLI only and 8.4.4 with FPM and CLI and your project requires FPM for instance). This also lays the ground to pick more variants (debug or zts for example)
1 parent e76629f commit 2147c05

File tree

4 files changed

+197
-4
lines changed

4 files changed

+197
-4
lines changed

store.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,21 @@ func (s *PHPStore) BestVersionForDir(dir string) (*Version, string, string, erro
151151
// break BC in minor versions, so we can't safely fall back.
152152
func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string, string, error) {
153153
warning := ""
154+
variant := ""
155+
156+
// Check if versionPrefix has a expectedVariants constraint, if so first do an
157+
// exact match lookup and fallback to a minor version check
158+
if pos := strings.LastIndexByte(versionPrefix, '-'); pos != -1 {
159+
variant = versionPrefix[pos+1:]
160+
versionPrefix = versionPrefix[:pos]
161+
}
154162

155163
// Check if versionPrefix is actually a patch version, if so first do an
156164
// exact match lookup and fallback to a minor version check
157165
if pos := strings.LastIndexByte(versionPrefix, '.'); pos != strings.IndexByte(versionPrefix, '.') {
158166
// look for an exact match, the order does not matter here
159167
for _, v := range s.versions {
160-
if v.Version == versionPrefix {
168+
if v.Version == versionPrefix && v.ForceVariant(variant) {
161169
return v, source, "", nil
162170
}
163171
}
@@ -173,7 +181,7 @@ func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string,
173181
// start from the end as versions are always sorted
174182
for i := len(s.versions) - 1; i >= 0; i-- {
175183
v := s.versions[i]
176-
if strings.HasPrefix(v.Version, versionPrefix) {
184+
if strings.HasPrefix(v.Version, versionPrefix) && v.ForceVariant(variant) {
177185
return v, source, warning, nil
178186
}
179187
}

store_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package phpstore
22

33
import (
44
"path/filepath"
5+
"sort"
56
"testing"
67
)
78

@@ -17,6 +18,20 @@ func TestBestVersion(t *testing.T) {
1718
}
1819
}
1920

21+
{
22+
v := "8.0.26"
23+
ver := NewVersion(v)
24+
ver.PHPPath = filepath.Join("/foo", v, "bin", "php")
25+
ver.FPMPath = filepath.Join("/foo", v, "bin", "php-fpm")
26+
store.addVersion(ver)
27+
28+
if !store.IsVersionAvailable(v) {
29+
t.Errorf("Version %s should be shown as available", v)
30+
}
31+
}
32+
33+
sort.Sort(store.versions)
34+
2035
{
2136
bestVersion, _, _, _ := store.bestVersion("8", "testing")
2237
if bestVersion == nil {
@@ -56,4 +71,17 @@ func TestBestVersion(t *testing.T) {
5671
t.Error("8.0.99 requirement should not trigger a warning")
5772
}
5873
}
74+
75+
{
76+
bestVersion, _, warning, _ := store.bestVersion("8.0-fpm", "testing")
77+
if bestVersion == nil {
78+
t.Error("8.0-fpm requirement should find a best version")
79+
} else if bestVersion.Version != "8.0.26" {
80+
t.Errorf("8.0-fpm requirement should find 8.0.26 as best version, got %s", bestVersion.Version)
81+
} else if bestVersion.serverType() != fpmServer {
82+
t.Error("8.0-fpm requirement should find an FPM expectedVariants")
83+
} else if warning != "" {
84+
t.Error("8.0-fpm requirement should not trigger a warning")
85+
}
86+
}
5987
}

version.go

Lines changed: 58 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ import (
3030
type serverType int
3131

3232
const (
33-
fpmServer serverType = iota
34-
cgiServer
33+
noServerType serverType = iota
3534
cliServer
35+
cgiServer
36+
fpmServer
3637
frankenphpServer
3738
)
3839

@@ -49,6 +50,8 @@ type Version struct {
4950
PHPdbgPath string `json:"phpdbg_path"`
5051
IsSystem bool `json:"is_system"`
5152
FrankenPHP bool `json:"frankenphp"`
53+
54+
typeOverride serverType
5255
}
5356

5457
func NewVersion(v string) *Version {
@@ -116,6 +119,9 @@ func (v *Version) serverType() serverType {
116119
if v.FrankenPHP {
117120
return frankenphpServer
118121
}
122+
if v.typeOverride != noServerType {
123+
return v.typeOverride
124+
}
119125
if v.FPMPath != "" {
120126
return fpmServer
121127
}
@@ -126,6 +132,56 @@ func (v *Version) serverType() serverType {
126132
return cliServer
127133
}
128134

135+
func (v *Version) ForceVariant(variant string) bool {
136+
if variant == "" {
137+
return true
138+
}
139+
140+
if !v.SupportsVariant(variant) {
141+
return false
142+
}
143+
144+
switch variant {
145+
case "cli":
146+
v.typeOverride = cliServer
147+
return true
148+
case "cgi":
149+
v.typeOverride = cgiServer
150+
return true
151+
case "fpm":
152+
v.typeOverride = fpmServer
153+
return true
154+
case "frankenphp":
155+
return true
156+
}
157+
158+
return false
159+
}
160+
161+
func (v *Version) SupportsVariant(variant string) bool {
162+
if variant == "" {
163+
return true
164+
}
165+
166+
serverVariant := v.serverType()
167+
if serverVariant == frankenphpServer {
168+
return variant == "frankenphp"
169+
}
170+
171+
if variant == "cli" {
172+
return true
173+
}
174+
175+
switch serverVariant {
176+
case cgiServer:
177+
return variant == "cgi"
178+
case fpmServer:
179+
return variant == "fpm"
180+
}
181+
182+
return false
183+
}
184+
129185
func (v *Version) setServer(fpm, cgi, phpconfig, phpize, phpdbg string) string {
130186
msg := fmt.Sprintf(" Found PHP: %s", v.PHPPath)
131187
fpm = filepath.Clean(fpm)

version_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
/*
2+
* Copyright (c) 2021-present Fabien Potencier <[email protected]>
3+
*
4+
* This file is part of Symfony CLI project
5+
*
6+
* This program is free software: you can redistribute it and/or modify
7+
* it under the terms of the GNU Affero General Public License as
8+
* published by the Free Software Foundation, either version 3 of the
9+
* License, or (at your option) any later version.
10+
*
11+
* This program is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU Affero General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU Affero General Public License
17+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
18+
*/
19+
20+
package phpstore
21+
22+
import (
23+
"testing"
24+
25+
"github.com/symfony-cli/dumper"
26+
)
27+
28+
func TestVersion_IsVariant(t *testing.T) {
29+
testCases := []struct {
30+
version *Version
31+
expectedVariants []string
32+
}{
33+
{
34+
version: func() *Version {
35+
v := NewVersion("8.1")
36+
v.FPMPath = "/usr/bin/php-fpm8.1"
37+
v.PHPPath = "/usr/bin/php-8.1"
38+
return v
39+
}(),
40+
expectedVariants: []string{"fpm", "cli"},
41+
},
42+
{
43+
version: func() *Version {
44+
v := NewVersion("8.2")
45+
v.CGIPath = "/usr/bin/php-cgi8.1"
46+
v.PHPPath = "/usr/bin/php-8.1"
47+
return v
48+
}(),
49+
expectedVariants: []string{"cgi", "cli"},
50+
},
51+
{
52+
version: func() *Version {
53+
v := NewVersion("8.3")
54+
v.PHPPath = "/usr/bin/php-8.3"
55+
return v
56+
}(),
57+
expectedVariants: []string{"cli"},
58+
},
59+
{
60+
version: func() *Version {
61+
v := NewVersion("8.4")
62+
v.PHPPath = "/usr/bin/frankenphp"
63+
v.FrankenPHP = true
64+
return v
65+
}(),
66+
expectedVariants: []string{"frankenphp"},
67+
},
68+
{
69+
version: func() *Version {
70+
v := NewVersion("7.4")
71+
v.PHPPath = "/usr/bin/php"
72+
v.FPMPath = "/usr/bin/php-fpm"
73+
return v
74+
}(),
75+
expectedVariants: []string{"fpm", "cli"},
76+
},
77+
}
78+
for _, testCase := range testCases {
79+
if !testCase.version.SupportsVariant("") {
80+
t.Error("version.SupportsVariant('') should return true, got false")
81+
}
82+
for _, variant := range testCase.expectedVariants {
83+
if !testCase.version.SupportsVariant(variant) {
84+
dumper.Dump(testCase.version)
85+
t.Errorf("version.SupportsVariant(%v) should return true, got false", variant)
86+
}
87+
}
88+
variantsLoop:
89+
for _, possibleVariant := range []string{"cli", "cgi", "fpm", "frankenphp", "franken"} {
90+
for _, variant := range testCase.expectedVariants {
91+
if variant == possibleVariant {
92+
continue variantsLoop
93+
}
94+
}
95+
96+
if testCase.version.SupportsVariant(possibleVariant) {
97+
t.Errorf("version.SupportsVariant(%v) should return false, got true", possibleVariant)
98+
}
99+
}
100+
}
101+
}

0 commit comments

Comments
 (0)