From b74dc4c4d99c0214ee897bc8f0f525653be2b6b4 Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Thu, 3 Jul 2025 08:42:24 +0200 Subject: [PATCH 1/3] refacto --- store.go | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/store.go b/store.go index e327167..acef76f 100644 --- a/store.go +++ b/store.go @@ -136,30 +136,19 @@ func (s *PHPStore) BestVersionForDir(dir string) (*Version, string, string, erro return s.fallbackVersion("") } -// bestVersion returns the latest patch version for the given major (X), minor (X.Y), or patch (X.Y.Z) -// version can be 7 or 7.1 or 7.1.2 -// non-symlinked versions have priority +// bestVersion returns the latest patch version for the given major (X), +// minor (X.Y), or patch (X.Y.Z). version can be 7 or 7.1 or 7.1.2. +// Non-symlinked versions have priority // If the asked version is a patch one (X.Y.Z) and is not available, the lookup -// will fallback to the last path version for the minor version (X.Y). +// will fallback to the last patch version for the minor version (X.Y). // There's no fallback to the major version because PHP is known to occasionally // break BC in minor versions, so we can't safely fall back. func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string, string, error) { warning := "" - isPatchVersion := false - pos := strings.LastIndexByte(versionPrefix, '.') - if pos != strings.IndexByte(versionPrefix, '.') { - if versionPrefix[pos+1:] == "99" { - versionPrefix = versionPrefix[:pos] - pos = strings.LastIndexByte(versionPrefix, '.') - } else { - isPatchVersion = true - } - } - // Check if versionPrefix is actually a patch version, if so first do an // exact match lookup and fallback to a minor version check - if isPatchVersion { + if pos := strings.LastIndexByte(versionPrefix, '.'); pos != strings.IndexByte(versionPrefix, '.') { // look for an exact match, the order does not matter here for _, v := range s.versions { if v.Version == versionPrefix { @@ -169,7 +158,9 @@ func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string, // exact match not found, fallback to minor version check newVersionPrefix := versionPrefix[:pos] - warning = fmt.Sprintf(`the current dir requires PHP %s (%s), but this version is not available: fallback to %s`, versionPrefix, source, newVersionPrefix) + if versionPrefix[pos+1:] != "99" { + warning = fmt.Sprintf(`the current dir requires PHP %s (%s), but this version is not available: fallback to %s`, versionPrefix, source, newVersionPrefix) + } versionPrefix = newVersionPrefix } From e76629f5a9a59ad1192d74bb93056760c061104a Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Thu, 3 Jul 2025 09:17:51 +0200 Subject: [PATCH 2/3] refacto --- store.go | 16 +++++++++++----- store_test.go | 17 ++++++++--------- version.go | 7 +++++++ 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/store.go b/store.go index acef76f..49fb89c 100644 --- a/store.go +++ b/store.go @@ -44,11 +44,8 @@ type PHPStore struct { // New creates a new PHP store func New(configDir string, reload bool, logger func(msg string, a ...interface{})) *PHPStore { - s := &PHPStore{ - configDir: configDir, - seen: make(map[string]int), - discoveryLogFunc: logger, - } + s := newEmpty(configDir, logger) + if reload { _ = os.Remove(filepath.Join(configDir, "php_versions.json")) } @@ -56,6 +53,15 @@ func New(configDir string, reload bool, logger func(msg string, a ...interface{} return s } +// newEmpty creates a new "empty" (without loading versions) PHP store +func newEmpty(configDir string, logger func(msg string, a ...interface{})) *PHPStore { + return &PHPStore{ + configDir: configDir, + seen: make(map[string]int), + discoveryLogFunc: logger, + } +} + // Versions returns all available PHP versions func (s *PHPStore) Versions() []*Version { return s.versions diff --git a/store_test.go b/store_test.go index e0ac43b..9bb10f9 100644 --- a/store_test.go +++ b/store_test.go @@ -6,12 +6,11 @@ import ( ) func TestBestVersion(t *testing.T) { - store := New("/dev/null", false, nil) + store := newEmpty("/dev/null", nil) for _, v := range []string{"7.4.33", "8.0.27", "8.1.2", "8.1.14", "8.2.1"} { - store.addVersion(&Version{ - Version: v, - PHPPath: filepath.Join("/foo", v, "bin", "php"), - }) + ver := NewVersion(v) + ver.PHPPath = filepath.Join("/foo", v, "bin", "php") + store.addVersion(ver) if !store.IsVersionAvailable(v) { t.Errorf("Version %s should be shown as available", v) @@ -23,7 +22,7 @@ func TestBestVersion(t *testing.T) { if bestVersion == nil { t.Error("8 requirement should find a best version") } else if bestVersion.Version != "8.2.1" { - t.Error("8 requirement should find 8.2.1 as best version") + t.Errorf("8 requirement should find 8.2.1 as best version, got %s", bestVersion.Version) } } @@ -32,7 +31,7 @@ func TestBestVersion(t *testing.T) { if bestVersion == nil { t.Error("8.1 requirement should find a best version") } else if bestVersion.Version != "8.1.14" { - t.Error("8.1 requirement should find 8.1.14 as best version") + t.Errorf("8.1 requirement should find 8.1.14 as best version, got %s", bestVersion.Version) } } @@ -41,7 +40,7 @@ func TestBestVersion(t *testing.T) { if bestVersion == nil { t.Error("8.0.10 requirement should find a best version") } else if bestVersion.Version != "8.0.27" { - t.Error("8.0.10 requirement should find 8.0.27 as best version") + t.Errorf("8.0.10 requirement should find 8.0.27 as best version, got %s", bestVersion.Version) } else if warning == "" { t.Error("8.0.10 requirement should trigger a warning") } @@ -52,7 +51,7 @@ func TestBestVersion(t *testing.T) { if bestVersion == nil { t.Error("8.0.99 requirement should find a best version") } else if bestVersion.Version != "8.0.27" { - t.Error("8.0.99 requirement should find 8.0.27 as best version") + t.Errorf("8.0.99 requirement should find 8.0.27 as best version, got %s", bestVersion.Version) } else if warning != "" { t.Error("8.0.99 requirement should not trigger a warning") } diff --git a/version.go b/version.go index 9ef3a76..24c345d 100644 --- a/version.go +++ b/version.go @@ -51,6 +51,13 @@ type Version struct { FrankenPHP bool `json:"frankenphp"` } +func NewVersion(v string) *Version { + return &Version{ + Version: v, + FullVersion: version.Must(version.NewVersion(v)), + } +} + type versions []*Version func (vs versions) Len() int { return len(vs) } From 2d61ff57be82e81b7d1eb95ed41d373e1ce7af22 Mon Sep 17 00:00:00 2001 From: Tugdual Saunier Date: Thu, 3 Jul 2025 09:58:01 +0200 Subject: [PATCH 3/3] feat: add support for flavor 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 flavors (debug or zts for example) --- store.go | 19 ++++++++--- store_test.go | 28 ++++++++++++++++ version.go | 72 +++++++++++++++++++++++++++++++++++++-- version_test.go | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 version_test.go diff --git a/store.go b/store.go index 49fb89c..d356e21 100644 --- a/store.go +++ b/store.go @@ -143,21 +143,32 @@ func (s *PHPStore) BestVersionForDir(dir string) (*Version, string, string, erro } // bestVersion returns the latest patch version for the given major (X), -// minor (X.Y), or patch (X.Y.Z). version can be 7 or 7.1 or 7.1.2. -// Non-symlinked versions have priority +// minor (X.Y), or patch (X.Y.Z). +// Version can be 7 or 7.1 or 7.1.2 and optionally suffixed with a flavor. +// Non-symlinked versions have priority. +// If the asked version contains a flavor (e.g. "7.4-fpm"), it will only accept +// versions supporting this flavor. // If the asked version is a patch one (X.Y.Z) and is not available, the lookup // will fallback to the last patch version for the minor version (X.Y). // There's no fallback to the major version because PHP is known to occasionally // break BC in minor versions, so we can't safely fall back. func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string, string, error) { warning := "" + flavor := "" + + // Check if versionPrefix has a expectedFlavors constraint, if so first do an + // exact match lookup and fallback to a minor version check + if pos := strings.LastIndexByte(versionPrefix, '-'); pos != -1 { + flavor = versionPrefix[pos+1:] + versionPrefix = versionPrefix[:pos] + } // Check if versionPrefix is actually a patch version, if so first do an // exact match lookup and fallback to a minor version check if pos := strings.LastIndexByte(versionPrefix, '.'); pos != strings.IndexByte(versionPrefix, '.') { // look for an exact match, the order does not matter here for _, v := range s.versions { - if v.Version == versionPrefix { + if v.Version == versionPrefix && v.ForceFlavorIfSupported(flavor) { return v, source, "", nil } } @@ -173,7 +184,7 @@ func (s *PHPStore) bestVersion(versionPrefix, source string) (*Version, string, // start from the end as versions are always sorted for i := len(s.versions) - 1; i >= 0; i-- { v := s.versions[i] - if strings.HasPrefix(v.Version, versionPrefix) { + if strings.HasPrefix(v.Version, versionPrefix) && v.ForceFlavorIfSupported(flavor) { return v, source, warning, nil } } diff --git a/store_test.go b/store_test.go index 9bb10f9..54cdc16 100644 --- a/store_test.go +++ b/store_test.go @@ -2,6 +2,7 @@ package phpstore import ( "path/filepath" + "sort" "testing" ) @@ -17,6 +18,20 @@ func TestBestVersion(t *testing.T) { } } + { + v := "8.0.26" + ver := NewVersion(v) + ver.PHPPath = filepath.Join("/foo", v, "bin", "php") + ver.FPMPath = filepath.Join("/foo", v, "bin", "php-fpm") + store.addVersion(ver) + + if !store.IsVersionAvailable(v) { + t.Errorf("Version %s should be shown as available", v) + } + } + + sort.Sort(store.versions) + { bestVersion, _, _, _ := store.bestVersion("8", "testing") if bestVersion == nil { @@ -56,4 +71,17 @@ func TestBestVersion(t *testing.T) { t.Error("8.0.99 requirement should not trigger a warning") } } + + { + bestVersion, _, warning, _ := store.bestVersion("8.0-fpm", "testing") + if bestVersion == nil { + t.Error("8.0-fpm requirement should find a best version") + } else if bestVersion.Version != "8.0.26" { + t.Errorf("8.0-fpm requirement should find 8.0.26 as best version, got %s", bestVersion.Version) + } else if bestVersion.serverType() != fpmServer { + t.Error("8.0-fpm requirement should find an FPM expectedFlavors") + } else if warning != "" { + t.Error("8.0-fpm requirement should not trigger a warning") + } + } } diff --git a/version.go b/version.go index 24c345d..227a4e5 100644 --- a/version.go +++ b/version.go @@ -30,12 +30,20 @@ import ( type serverType int const ( - fpmServer serverType = iota - cgiServer + noServerType serverType = iota cliServer + cgiServer + fpmServer frankenphpServer ) +const ( + FlavorCLI string = "cli" + FlavorCGI string = "cgi" + FlavorFPM string = "fpm" + FlavorFrankenPHP string = "frankenphp" +) + // Version stores information about an installed PHP version type Version struct { FullVersion *version.Version `json:"-"` @@ -49,6 +57,8 @@ type Version struct { PHPdbgPath string `json:"phpdbg_path"` IsSystem bool `json:"is_system"` FrankenPHP bool `json:"frankenphp"` + + typeOverride serverType } func NewVersion(v string) *Version { @@ -113,9 +123,14 @@ func (v *Version) IsFrankenPHPServer() bool { } func (v *Version) serverType() serverType { + // FrankenPHP is a special case as it will not support several server types + // for a single installation. if v.FrankenPHP { return frankenphpServer } + if v.typeOverride != noServerType { + return v.typeOverride + } if v.FPMPath != "" { return fpmServer } @@ -126,6 +141,59 @@ func (v *Version) serverType() serverType { return cliServer } +func (v *Version) ForceFlavorIfSupported(flavor string) bool { + if flavor == "" { + return true + } + + if !v.SupportsFlavor(flavor) { + return false + } + + switch flavor { + case FlavorCLI: + v.typeOverride = cliServer + return true + case FlavorCGI: + v.typeOverride = cgiServer + return true + case FlavorFPM: + v.typeOverride = fpmServer + return true + case FlavorFrankenPHP: + // FrankenPHP installations does not support multiple flavors so there's + // no need to set the typeOverride. + return true + } + + return false +} + +func (v *Version) SupportsFlavor(flavor string) bool { + if flavor == "" { + return true + } + + serverFlavor := v.serverType() + if serverFlavor == frankenphpServer { + return flavor == FlavorFrankenPHP + } + + // CLI flavor is always supported + if flavor == FlavorCLI { + return true + } + + switch serverFlavor { + case cgiServer: + return flavor == FlavorCGI + case fpmServer: + return flavor == FlavorFPM + } + + return false +} + func (v *Version) setServer(fpm, cgi, phpconfig, phpize, phpdbg string) string { msg := fmt.Sprintf(" Found PHP: %s", v.PHPPath) fpm = filepath.Clean(fpm) diff --git a/version_test.go b/version_test.go new file mode 100644 index 0000000..f324c73 --- /dev/null +++ b/version_test.go @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2021-present Fabien Potencier + * + * This file is part of Symfony CLI project + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package phpstore + +import ( + "testing" +) + +func TestVersion_SupportsFlavor(t *testing.T) { + testCases := []struct { + version *Version + expectedFlavors []string + }{ + { + version: func() *Version { + v := NewVersion("8.1") + v.FPMPath = "/usr/bin/php-fpm8.1" + v.PHPPath = "/usr/bin/php-8.1" + return v + }(), + expectedFlavors: []string{FlavorFPM, FlavorCLI}, + }, + { + version: func() *Version { + v := NewVersion("8.2") + v.CGIPath = "/usr/bin/php-cgi8.1" + v.PHPPath = "/usr/bin/php-8.1" + return v + }(), + expectedFlavors: []string{FlavorCGI, FlavorCLI}, + }, + { + version: func() *Version { + v := NewVersion("8.3") + v.PHPPath = "/usr/bin/php-8.3" + return v + }(), + expectedFlavors: []string{FlavorCLI}, + }, + { + version: func() *Version { + v := NewVersion("8.4") + v.PHPPath = "/usr/bin/frankenphp" + v.FrankenPHP = true + return v + }(), + expectedFlavors: []string{FlavorFrankenPHP}, + }, + } + for _, testCase := range testCases { + if !testCase.version.SupportsFlavor("") { + t.Error("version.SupportsFlavor('') should return true, got false") + } + for _, flavor := range testCase.expectedFlavors { + if !testCase.version.SupportsFlavor(flavor) { + t.Errorf("version.SupportsFlavor(%v) should return true, got false", flavor) + } + } + flavorLoop: + for _, possibleFlavor := range []string{FlavorCLI, FlavorCGI, FlavorFPM, FlavorFrankenPHP} { + for _, flavor := range testCase.expectedFlavors { + if flavor == possibleFlavor { + continue flavorLoop + } + } + + if testCase.version.SupportsFlavor(possibleFlavor) { + t.Errorf("version.SupportsFlavor(%v) should return false, got true", possibleFlavor) + } + } + } +}