humbug / phar-updater

A thing to make PHAR self-updates easy and secure
BSD 3-Clause "New" or "Revised" License
368 stars 27 forks source link

github strategy is using the wrong api for packagist #10

Closed ppetermann closed 6 years ago

ppetermann commented 9 years ago

currently the strategy is using: https://packagist.org/packages/vendor/package.json which is a json representation of the package site and has a pretty long cache timer (1h),

while composer uses the actual API: https://packagist.org/p/vendor/package.json which has only 1m cache time

phar-updater should be using the sami API as composer here.

aik099 commented 8 years ago

Does it matter that much? We'll be putting less load on Packagist website and I don't think it matters that much for users to be able to update to new releases within 1st hour of it's creation.

ppetermann commented 8 years ago

to me that actually makes a difference - when i tag a release, usually i want to use the stuff in a timely manner, and not wait an hour

aik099 commented 8 years ago

Different people, different use cases.

Considering there is only 1 maintainer on a project (the issue wasn't reviewed in 1 year) and that maintainer isn't looking at GitHub much lately I'm not sure what can be done here.

You can use regular approach:

  1. fork a repo
  2. create branch with needed changes
  3. use that repo/branch in your composer.json
ppetermann commented 8 years ago

thats a dirty solution, the right approach would be to make a PR so @padraic can merge it in.

aik099 commented 8 years ago

Of course, but issue wasn't touched in a year. What you think are the odds of that PR being merged.

I like FOSS, but when lead project maintainer have no time to maintain it and haven't appointed a replacement maintainer then project is as good as dead and people would start to make changes in forks to at least get it going for them.

aik099 commented 8 years ago

The #9 for example is a show stopper for all PHP < 5.6 users. And that issue wasn't addressed as well.

ppetermann commented 8 years ago

to be fair, while i did still report #9 , it already was closing in on php 5.5's end-of-support date, and is now almost end-of-life, so < 5.6 shouldn't be used anymore anyways, so thats not THAT big of a deal.

padraic commented 7 years ago

Composer and Packagist are interlinked in one sense, so I was trying not to be a jerk by abusing their resources any more than necessary. Maybe if included as an option for those who need/prefer it?

maks-rafalko commented 7 years ago

I'm also not comfortable with a current implementation with 1h delay as @ppetermann, so at least an option would be very useful to choose between two URL.

But going further, why not just use Github API? Why packagist is envolved in the github strategy?

I mean, this github's API can be used instead: https://api.github.com/repos/humbug/humbug/releases

padraic commented 7 years ago

@borNfreee The naming is probably not brilliant here, but the strategy is more accurately Packagist w/ Github Releases as a download source. I should describe it better in the README.

The intent is fairly simple. You have an issue in a package release, so you yank it on Packagist, but forget to delete the Github Release (or intentially keep it as a reference). The self-update will currently work just as Composer will since we interrogate Packagist. If we instead took Github as the primary authority for package releases, we would miss this and potentially update to problematic releases.

ppetermann commented 7 years ago

I agree that packagist is the right source for this. I just think the api would be more useful

webflo commented 7 years ago

Looks like Packagist.com increased max-age. s-maxage is currently set to 12 hours.

curl -I https://packagist.org/packages/webflo/drush-shim.json

HTTP/1.1 200 OK
Server: nginx
Content-Type: application/json
Connection: keep-alive
Vary: Accept-Encoding
Cache-Control: public, s-maxage=43200
Strict-Transport-Security: max-age=31104000
X-Frame-Options: DENY
Access-Control-Allow-Origin: *
Date: Tue, 25 Jul 2017 15:39:12 GMT
webflo commented 7 years ago

Created a PR to use the other URL. https://github.com/humbug/phar-updater/pull/51

padraic commented 7 years ago

I've approved @webflo's PR. We have a stack of other PRs and such for next major/minor release, so I'll let @theofidry chip in on whether this should be taken to the 1.0 branch for a patch release given the 12 hour switch.

theofidry commented 7 years ago

I can have a look at it this weekend, can't do before

padraic commented 7 years ago

@theofidry np :)

webflo commented 7 years ago

@padraic @theofidry Thanks, for the response. I fix the test later today.