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

please remove "ius" from release field #13

Open
carlwgeorge opened this issue Apr 5, 2017 · 14 comments
Open

please remove "ius" from release field #13

carlwgeorge opened this issue Apr 5, 2017 · 14 comments

Comments

@carlwgeorge
Copy link

First off, thanks for choosing to base your phalcon RPM packages off of the IUS php70u packages. We love seeing users and developers benefit from our work.

I do have one small request. Please remove the "ius" string from your release field.

%global repo_vendor ius
Release: 1.%{repo_vendor}%{?dist}

Naming the packages with the same %{php_base} as ours is appropriate, but including "ius" in the release field is misleading. Today a user came to our IRC channel asking questions about php70u-phalcon. I would suggest setting repo_vendor to phalcon or removing it entirely.

@sergeyklay
Copy link
Member

We need to generate RPMs like:

  • php55u-phalcon-3.0.4-1.ius.el7.centos.x86_64.rpm (Remi/IUS)
  • ...
  • php55w-phalcon-3.0.4-1.w6.el7.centos.x86_64.rpm (Webtatic)
  • ...

Also take a look at https://github.com/phalcongelist/packagecloud/blob/master/builder/patching.mk#L41

@carlwgeorge
Copy link
Author

  • Why do you say "Remi/IUS"? They are separate repos and should not be used together.
  • I don't see any reference of Webtatic in the repository.
  • You say you "need" to set the repo_vendor tag, but why? Your phalcon RPM packages don't come from IUS or Webtatic, so they shouldn't have those strings in the release.

@virgofx
Copy link
Contributor

virgofx commented Apr 5, 2017

We should be able to drop the name on the vendor tag provided we can uniquely identify similar versions of Phalcon across different platform vendors. That means we'll need to get a matrix to see if the prefix name portion is unique across all vendors (e.g. php56u [IUS]). Otherwise, may need to adjust the vendor tag to be "phalcon-ius".

@carlwgeorge Webtatic isn't done yet here ... nor is Remi which is why it's not shown in source.

@sergeyklay
Copy link
Member

As you know guys, we store packages in Packagecloud. So we have to use different file names to be able push package to cloud.

@sergeyklay
Copy link
Member

sergeyklay commented Apr 5, 2017

Well, I agree with @virgofx we can use format like

  • php55u-phalcon-3.0.4-1.el7.centos.x86_64.rpm
  • php55w-phalcon-3.0.4-1.el7.centos.x86_64.rpm

@sergeyklay
Copy link
Member

Probably we should create a more universal RPM-spec file and just use format like php55-phalcon-3.0.4-1.el7.centos.x86_64.rpm without repo vendor at all.

Cc: @virgofx

@virgofx
Copy link
Contributor

virgofx commented Apr 6, 2017

The problem with the last method @sergeyklay is that Linux on the non-debian side for PHP is very specific which means the playground for PHP will either be one of the major PHP RPM providers so I don't think we can do a universal RPM spec ... it needs to be consistent with each of the main RPM providers (IUS, Webtatic, Remi, etc) This holds true for the dependencies (e.g. php56u-phalcon should require php56u and php56u-common)

@carlwgeorge
Copy link
Author

@sergeyklay That approach would probably be ideal. You can accomplish that by taking advantage of virtual provides of the packages.

-Requires: %{php_base}(zend-abi) = %{zend_apiver}
-Requires: %{php_base}(api) = %{php_apiver}
+Requires: php(zend-abi) = %{php_zend_api}
+Requires: php(api) = %{php_core_api}

That should ensure compatibility with IUS, Webtatic, Remi, and any other providers who follow the pattern Fedora sets (which should be all of them).

@sergeyklay
Copy link
Member

@carlwgeorge I start to refactor it (70acb45). Feel free to send PR to the dev branch.

@sergeyklay
Copy link
Member

@virgofx Is there any changes/ideas related to this issue

@virgofx
Copy link
Contributor

virgofx commented May 2, 2017

Not yet. I need to spend some time with it. I still have it on my list to get everything working with all Linux distros just been super busy at work :/

@carlwgeorge
Copy link
Author

This is still an issue. Please remove the "ius" string from your release field.

@virgofx
Copy link
Contributor

virgofx commented Mar 15, 2018

Since IUS doesn't build this package, we do ... What's the issue? This package is targeted specifically for use in combination with other php* IUS packages.

@carlwgeorge
Copy link
Author

It confuses users, who mistake your packages as packages built by IUS, and then come to us to report issues or ask questions. I think it's great that you're building off IUS packages, I'm just asking for a simple change to the release field. The php??u- prefix is enough to indicate that it's intended to be used with IUS packages. How about setting repo_vendor to phalcon?

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

3 participants