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

Some important files dropped from the releases #201

Closed
orlitzky opened this issue May 14, 2017 · 11 comments
Closed

Some important files dropped from the releases #201

orlitzky opened this issue May 14, 2017 · 11 comments

Comments

@orlitzky
Copy link
Contributor

Hello, while trying to upgrade the Gentoo package for php-redmine-api, I noticed that some of the files we usually install have gone missing from the releases. This is due to the .gitattributes file, so I know it was intentional, but I'd like to ask you to please bring some of them back =)

These are all useful to an end-user:

/test/            export-ignore
README.markdown   export-ignore
phpunit.xml.dist  export-ignore
example.php       export-ignore

We run the tests before installing the package, to ensure that there are no incompatibilities on the user's system. That requires the test directory and phpunit.xml.dist file. We also install the documentation, which in this case consists of simply README.markdown and example.php. Without those two files, it's hard to figure out how to use the library without internet access.

To pre-empt the question of why we can't just get them from git,

  • End users may not have git installed, and requiring them to do so brings in a lot of stuff they don't need/want.
  • I can't digitally sign a "git clone" like I can the sha256 of your release tarball. So we lose a little bit of security that way.
  • We have a mirror system that keeps a copy of the souce code, in case github goes down. We're set up to mirror tarballs, but not entire git repos. (This is somewhat related to the previous item, since our mirrors are not 100% trusted.)

tl;dr having those files back would make php-redmine-api easier to package for me.

@Art4
Copy link
Collaborator

Art4 commented May 14, 2017

I'm with you for the README.markdown as a minimal documentation or reference to the source code. But personally I'm against the other files for this reasons:

  1. You should use the source code in your case, not the dist code.
  2. The dists (tarballs) are for projects/libraries that builds on this library. They don't need the tests, example.php, etc and save traffic and time without these files.
  3. If the tests are in the dists the .gitattribute wouldn't make much sense any more because there would only be some single files excluded.
  4. You should only sign the code you've tested. My recommendation would be: clone the code, test it, build a tarball from it and then sign it. This way you have full control over your package and your users won't need git.

@orlitzky
Copy link
Contributor Author

Who is example.php for then, if not for the authors of the projects and libraries that build on php-redmine-api?

Likewise: I use php-redmine-api for a project of mine. When I install it, I want to run the tests, because I want to be sure that it works on my machine (with my version of PHP, with the other libraries on my system, etc.)

It's always easier for the people who don't want those things to remove them than the other way around. Your recommendation is essentially that I fork the project to remove four lines from a text file. I can't do that for every package that I maintain.

@Art4
Copy link
Collaborator

Art4 commented May 14, 2017

I don't know your workflow for a Gento package, but my workflow for a PHP project works like this:

I'm using Composer for dependency management. To develop my project I'm installing php-redmine-api from source with --prefer-source. This clones php-redmine-api into my vendor folder. This way I can run the tests to be sure that it works on my machine. Than I can use the example.php for the develoment (or I simply read it on Github).

On the production server I don't need the tests, so I'm installing the dependencies of my project with --prefer-dist. This downloads the tarballs and unzips them and thanks to the missing tests I'm saving traffic and time. On production it won't save me anything if I had to remove the tests after downloading the tarballs.

Your recommendation is essentially that I fork the project to remove four lines from a text file. I can't do that for every package that I maintain.

No, my recommendation is that you run the tests on a clone (not a fork). You don't have to modify any code and (depending on your tests) you can automate this tasks completely.

Example (simple draft - untestet):

#!/bin/bash

# clone the repo
git clone https://github.com/kbsali/php-redmine-api
cd php-redmine-api
git checkout v1.5.14

# run the tests
composer install
./vendor/bin/phpunit
rm -r ./vendor

# create the tarball
rm -r ./.git
cd ..
tar -cvfz php-redmine-api.tgz php-redmine-api/

# sign the tarball
openssl dgst -sha256 -sign <private-key> -out ./php-redmine-api.sign.sha256 php-redmine-api.tgz

@kbsali
Copy link
Owner

kbsali commented May 24, 2017

sorry, not very reactive those days! :/

I really don't mind putting those files back in the repo... not sure if we should include them ALL though?

As usual huge thanks to @Art4 for taking the time to answer clearly!

@orlitzky, does @Art4 's answer work for you?

@Art4
Copy link
Collaborator

Art4 commented May 24, 2017

@orlitzky I've dived deeper into your issue and the Gentoo package building process. This is all new to me, but I've found some interesting points.

I've found your Gentoo package (https://packages.gentoo.org/packages/dev-php/php-redmine-api) and I understand that you have an ebuild file (https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-php/php-redmine-api/php-redmine-api-1.5.9.ebuild) for your build process.

As far as I understand you can use git in the ebuild thanks to git-r3.class (see https://devmanual.gentoo.org/eclass-reference/git-r3.eclass/index.html). So you should only have to:

  • load git-r3.class with inherit git-r3(?)
  • remove the SRC_URI variable
  • set the github-url with the EGIT_REPO_URI
  • set the version for checkout with EGIT_COMMIT

Not sure if this will work but this seems to be a legit way to use the source code instead of the dist files.

@orlitzky
Copy link
Contributor Author

@Art4's last comment is correct and is what I'll have to do if the tests and documentation remain absent from the tarball releases. However, there are a number of non-obvious downsides to that approach (from my first comment):

  • End users may not have git installed, and requiring them to do so brings in a lot of stuff they don't need/want.
  • I can't digitally sign a "git clone" like I can the sha256 of your release tarball. So we lose a little bit of security that way.
  • We have a mirror system that keeps a copy of the souce code, in case github goes down. We're set up to mirror tarballs, but not entire git repos. (This is related to the previous item, since our mirrors are not 100% trusted.)

So the choice I'm facing is to adopt those downsides, or to omit the tests and documentation from the package (also bad). It would be better for us to have everything in the tarball, but I don't want to beat this to death.

@kbsali
Copy link
Owner

kbsali commented May 29, 2017

@orlitzky i really don't mind getting rid of the gitattributes to be honnest, i was just trying to follow the good practices... but the "gain" is minimal.

@Art4 what do you say? :)

@Art4
Copy link
Collaborator

Art4 commented May 29, 2017

The upsites of the .gitattribute don't weight as much as the downsites for @orlitzky. It's not his fault that the Gentoo build process isn't as flexible as needed.

The good practice of the .gitattributes doesn't cover use cases out of composer/PHP, so I would go with @orlitzky's suggestion and drop the files from .gitattribute.

@kbsali
Copy link
Owner

kbsali commented May 29, 2017

@Art4 the "weight gain" is so small for that project that this argument is not really relevant (although true! :) ).
Ok i'll remove them!

@kbsali
Copy link
Owner

kbsali commented May 29, 2017

@orlitzky
Copy link
Contributor Author

Thanks guys. I ran into some more trouble with the version bump from commit 83e46c2, but this will make things easier for me.

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

No branches or pull requests

3 participants