Skip to content
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

Closed
kelunik opened this issue Jan 5, 2017 · 39 comments
Closed

(php) Fallback to archive url for v5.3.x, v5.4.x, v5.5.x #510

kelunik opened this issue Jan 5, 2017 · 39 comments
Assignees

Comments

@kelunik
Copy link

kelunik commented Jan 5, 2017

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.

@photodude
Copy link
Contributor

photodude commented Jan 25, 2017

Based on main installer script conditional and the helper function that should be happening, maybe something is not correct with the AddArchivePathToUrl() function...
https://github.com/chocolatey/chocolatey-coreteampackages/blob/master/automatic/php/tools/helpers.ps1#L15-L22

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

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

cinst --ignore-checksums -r -y php --version ((choco search php --exact --all-versions -r | select-string -pattern 5.5 | Select-Object -first 1) -replace '[php|]','')

@photodude
Copy link
Contributor

With archived versions Select-Object -first 1 might be the wrong choice

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)

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

With archived versions Select-Object -first 1 might be the wrong choice

Why?

@photodude
Copy link
Contributor

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.

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

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 http://windows.php.net/downloads/releases/php-5.5.16-nts-Win32-VC11-x64.zip either doesn't exist or is unauthorized, server returned 404 Not Found.

@AdmiringWorm
Copy link
Member

@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.
Unfortunately I don't believe there will be added any fixes to any versions earlier than that one, as it's no longer supported by the developers.

As such, I'm also going to close this issue as it will not be implemented for earlier versions.

@photodude
Copy link
Contributor

@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.

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

@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.

@ferventcoder
Copy link
Contributor

it's still important to run tests for the older still supported majors on all supported versions.

So much this comment. I've lived this in Ruby land, so I completely understand and support this idea.

@ferventcoder
Copy link
Contributor

@AdmiringWorm could we give it another look?

@AdmiringWorm
Copy link
Member

Like I said it's not about php supporting fixes it's about us verifying our code runs on a particular version of php.

Ah yes, sorry didn't think of it that way, guess I closed this a little too soon without taking that into consideration.

@AdmiringWorm could we give it another look?

Certainly, we can take another look at this.
I'm not entirely sure how we should handle this though.
Just reusing the current script with the necessary url changes, or create a new one just using the archive url?

@AdmiringWorm
Copy link
Member

I'll look into providing some updated packages for the old versions.
For now I only plan to add the latest 5.5.x version, but if there is any other versions anyone would like then please let me know.

/cc @kelunik @photodude @ferventcoder

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

I'm fine with having one version, which ever it is.

@photodude
Copy link
Contributor

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.

@AdmiringWorm
Copy link
Member

@photodude 👍
I'll look into adding updated versions of 5.3.x and 5.4.x as well.

@AdmiringWorm
Copy link
Member

AdmiringWorm commented Jan 25, 2017

@photodude Am I correct to asume that php v.5.3.x doesn't support 64bit?
Perhaps v5.4.x as well?

@kelunik
Copy link
Author

kelunik commented Jan 25, 2017

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 --force-x64 or what ever the flag is called.

@photodude
Copy link
Contributor

photodude commented Jan 25, 2017

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.
the following is from http://windows.php.net/

The x64 builds of PHP 5 for Windows are experimental, and do not provide 64-bit integer or large file support.

Looking at the archive list php 5.5 is the first version to have a x64 package included

PHP 7 provides full 64-bit support. The x64 builds of PHP 7 support native 64-bit integers, LFS, 64-bit memory_limit and much more.

Additionally, if your using IIS on windows you need the NTS version as only Apache is TS
see this note from http://windows.php.net/

If you are using PHP as FastCGI with IIS you should use the Non-Thread Safe (NTS) versions of PHP. With Apache you have to use the Thread Safe (TS) versions of PHP.

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.

@AdmiringWorm
Copy link
Member

@kelunik said

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 --force-x64 or what ever the flag is called.

as far as I know, by default a package uses 64bit unless a --forcex86 is used (on a 64bit OS)

@photodude

As far as I'm aware even in php 5.5 and 5.6 the x64 support is experimental. all of those versions should all default to x86.

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.

Additionally if your using IIS on windows you need the NTS version as only Apache is TS
see this note from http://windows.php.net/

The existing packages already uses the NTS version, so i'm going to keep using that.

@photodude
Copy link
Contributor

@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)

@AdmiringWorm
Copy link
Member

@photodude Currently there is no way to foce the TS version.
But I can certainly look into that as well.

@AdmiringWorm AdmiringWorm changed the title (php) Fallback to archive versions (php) Fallback to archive url for v5.3.x, v5.4.x, v5.5.x Jan 25, 2017
@AdmiringWorm AdmiringWorm self-assigned this Jan 25, 2017
@photodude
Copy link
Contributor

@AdmiringWorm I was just looking at choco list php --exact --all-versions seems PHP 5.4 is missing from the list and PHP 7.0.9 is the last php7 version listed although the package list says 7.0.13 is available and PHP 7.0.15 is available in the win releases http://windows.php.net/downloads/releases/

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/

@AdmiringWorm
Copy link
Member

@photodude Thanks, I'm aware of it.
Both of those are on my TODO list, but want to complete #548 before I do.

@ChadSikorra
Copy link

ChadSikorra commented Feb 5, 2017

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:

php package files install completed. Performing other installation steps.
Using archive urls
ERROR: You cannot call a method on a null-valued expression.
The install of php was NOT successful.
Error while running 'C:\ProgramData\chocolatey\lib\php\tools\chocolateyInstall.ps1'.

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.

@AdmiringWorm
Copy link
Member

Basically the logic is currently as follows.

  1. Read the main release page urls from downloadInfo.csv
  2. Test if installer is located at that url
    • If it doesn't exist, change it to the archive url
  3. Pass the url to the package arguments.

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.

@ChadSikorra

@AdmiringWorm
Copy link
Member

@kelunik @photodude

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.

@AdmiringWorm
Copy link
Member

Forgot about closing this one, all requested versions should be available now.
Let me know if there are any remaining

@photodude
Copy link
Contributor

@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

@AdmiringWorm
Copy link
Member

AdmiringWorm commented Feb 15, 2017

@photodude yes, that's intentional to allow side-by-side installation.
If you absolutely need for it to install to the /tools/php directory you can pass
--params '""/InstallDir:C:\tools\php"""' when calling choco install

@photodude
Copy link
Contributor

@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"
        }

@AdmiringWorm
Copy link
Member

@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.
For 5.6.x you're installing the 5.6.9 version while the latest is 5.6.30 (you're using default sorting, instead of version sorting)
For 7.0.x you're install the 7.0.9 version while the latest is 7.0.16 (same sorting problem).

As I said, you'll need to specify the InstallDir: parameter if you wish to continue installing them to C:\tools\php (when sorting is fixed, that is)

@photodude
Copy link
Contributor

photodude commented Feb 15, 2017

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.

@AdmiringWorm
Copy link
Member

If i remember correctly, then it should be enough to add

| sort { [version]$_ } -Descending

right before you have | Select-Object.
You'll need to test that though, as I'm not completely sure.

@photodude
Copy link
Contributor

thanks @AdmiringWorm I'll give it a go. I hope that kind of detail can be added to the documentation.

@AdmiringWorm
Copy link
Member

just tested out sorting by version,
you'll need to swap out the choco search command with the following:

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?

@photodude
Copy link
Contributor

photodude commented Feb 15, 2017

both that php now defaults to install to versioned directories, and how to do choco search with sorting by version

@AdmiringWorm
Copy link
Member

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.

@photodude
Copy link
Contributor

thanks @AdmiringWorm the changes have solved my problems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants