-
Notifications
You must be signed in to change notification settings - Fork 383
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(php) Fallback to archive url for v5.3.x, v5.4.x, v5.5.x #510
Comments
Based on main installer script conditional and the helper function that should be happening, maybe something is not correct with the AddArchivePathToUrl() function... |
I guess it works for newer releases then, but not old ones? The one I'm specifically having issues with is PHP 5.5 installations for CI purposes. https://github.com/amphp/dns/blob/master/appveyor.yml#L39
|
With archived versions I also wonder if choco needs to take into consideration nts vs ts versions of php in the archive (I believe iis needs the nts version as it's not thread safe where as apache can us the ts version of php) |
Why? |
Look at the php 5.5 items in the archive, which minor version will select object first pick? That's what I see might be a possible problem. |
I don't care about the actual minor version being used for CI purposes, but the one that's chosen doesn't install, because it can't find it. https://ci.appveyor.com/project/kelunik/dns/build/job/9k7sfw429rea1lh7 doesn't show that exact error currently, but did before and also locally I get that |
@kelunik I looked through the existing versions of the php package, and it seems the fallback to using the archives url was implemented in version 5.6.2. As such, I'm also going to close this issue as it will not be implemented for earlier versions. |
@AdmiringWorm for ci purposes it doesn't matter if php no longer supports the version being tested against. I work with projects that still have php 5.3 as a supported version to run their package on. Usually it's corporate intranets that still run those old versions. It will be another 2 years before we completely stop supporting/writing code that runs on php 5.3 and we only have a code base running on php 5.6+. Like I said it's not about php supporting fixes it's about us verifying our code runs on a particular version of php. |
@AdmiringWorm Thanks for checking, I'm glad it works on all newer versions. While PHP 5.5 has reached its EOL and even 5.6 being already in security-only mode, there are still a lot of packages supporting it. Whether that's good or bad is another discussion. We (amphp) will completely drop PHP 5 support in the next version, as does Symfony and other major frameworks, but I think it's still important to run tests for the older still supported majors on all supported versions. That said, our major CI platform is Travis, so I can live without PHP 5.5 on Windows. |
So much this comment. I've lived this in Ruby land, so I completely understand and support this idea. |
@AdmiringWorm could we give it another look? |
Ah yes, sorry didn't think of it that way, guess I closed this a little too soon without taking that into consideration.
Certainly, we can take another look at this. |
I'll look into providing some updated packages for the old versions. |
I'm fine with having one version, which ever it is. |
For Joomla-CMS we would like just the last version of 5.3, 5.4, and 5.5. On a side note, like @kelunik and amphp we also primarily test with Travis CI. Additionally, our next major version will only support PHP 5.6+. Our current version nearing release 3.7 will be supported for 2 years and has a minimum php of 5.3, so any older PHP versions we can bring into Appveyor with choco for CI testing on windows is helpful but we can make do if it can't be done. |
@photodude 👍 |
@photodude Am I correct to asume that php v.5.3.x doesn't support 64bit? |
At least there aren't any builds for it. I guess we could just fall back to x86 there and error out in case of a |
As far as I'm aware even in php 5.5 and 5.6 the x64 support is experimental. all of those versions should default to x86.
Looking at the archive list php 5.5 is the first version to have a x64 package included
Additionally, if your using IIS on windows you need the NTS version as only Apache is TS
Of course, it's important to remember there are usually no issues running an x86 package on an x64 platform.... the only issue is an x86 platform can never run an x64 package. |
@kelunik said
as far as I know, by default a package uses 64bit unless a
Unfortunately the existing 5.5 and 5.6 packages defaults to 64bit, and I want to keep them consistent so that's what I'm going to do as well.
The existing packages already uses the NTS version, so i'm going to keep using that. |
@AdmiringWorm I'm all for consistency in default settings, especially when there are options to override with --forcex86. So defaulting to NTS is fine, I hope that there is is a way to force the TS version of PHP if that is needed (we are focusing on testing with IIS on windows so NTS is fine for us especially as a default) |
@photodude Currently there is no way to foce the TS version. |
@AdmiringWorm I was just looking at any tips on how to get the correct latest minor version of the packages that is available on http://windows.php.net/downloads/releases/ or http://windows.php.net/downloads/releases/archives/ |
@photodude Thanks, I'm aware of it. |
Perhaps I'm overlooking something and need to look closer, but the issue appears to be that the downloadInfo.csv file is generating all of its URLs from the main releases page. Then if it doesn't exist there it attempts to use the archive URL, but the value it passes to that method will never work because it wouldn't be in the downloadInfo if it is already in the archive. At least the logic error seems to be the case based on my AppVeyor script that attempts to use this package:
The null value expression being the case where it tried to pass the non-existent URL to the archives url method. What it should probably do is generate the downloadInfo.csv from both the releases page and the archive page. Or just pass the version/platform to the archive url method and figure it out based on just that. |
Basically the logic is currently as follows.
The reason it's not working for the existing package is because they used a non-existing url variable, this is fixed in the latest version in this repo (not pushed) and awaiting review to fix it in existing packages. |
v5.3.29, v5.4.45 and 5.5.28 have just been pushed and should be available on chocolatey.org soon. the latest v5.6.x and 7.0.x will come later. |
Forgot about closing this one, all requested versions should be available now. |
@AdmiringWorm I'm now seeing PHP 7.1 always installing in a versioned directory, previously all php would default to /tools/php see this appveyor build https://ci.appveyor.com/project/joomla/joomla-cms/build/1.0.643/job/2b5vfkcwoofnp618 nothing has changed on our side, just the recent chocolatey changes |
@photodude yes, that's intentional to allow side-by-side installation. |
@AdmiringWorm this is the script for the install with cinst on appveyor which installs correctly for php 5.6 and php 7.0 to C:\tools\php it's only php 7.1 that is installing to a different directory C:\tools\php71. - php_ver_target: 5.6
- php_ver_target: 7.0
- php_ver_target: 7.1
- ps: >-
If ($env:php_ver_target -eq "5.6") {
appveyor-retry cinst --ignore-checksums -y --forcex86 php --version ((choco search php --exact --all-versions -r | select-string -pattern $Env:php_ver_target | Select-Object -first 1) -replace '[php|]','')
$VC = "vc11"
$PHPBuild = "x86"
} Else {
appveyor-retry cinst --ignore-checksums -y php --version ((choco search php --exact --all-versions -r | select-string -pattern $Env:php_ver_target | Select-Object -first 1) -replace '[php|]','')
$VC = "vc14"
$PHPBuild = "x64"
} |
@photodude they all are supposed to install to a versioned folder, and I can see why 5.6.x and 7.0.x isn't. As I said, you'll need to specify the |
Ok, I'll make the install dir change. What do I change to use version sorting? I'm not seeing anything in the documentation or the source change that allowed version sorting to explain what to do. |
If i remember correctly, then it should be enough to add | sort { [version]$_ } -Descending right before you have |
thanks @AdmiringWorm I'll give it a go. I hope that kind of detail can be added to the documentation. |
just tested out sorting by version, choco search php --exact --all-versions -r | select-string -pattern $env:php_ver_target | sort { [version]($_ -split '\|' | select -last 1) } -Descending | Select-Object -first 1 regarding the documentation, do you mean mentioning it now installs to versioned directories? |
both that php now defaults to install to versioned directories, and how to do choco search with sorting by version |
I'm not sure about adding the sorting by version, but I'll see if I can't get around to adding a note about versioned directories. |
thanks @AdmiringWorm the changes have solved my problems |
The PHP installation should fallback to http://windows.php.net/downloads/releases/archives/ instead of http://windows.php.net/downloads/releases/ if the version cannot be found there.
The text was updated successfully, but these errors were encountered: