Skip to content

Make doctrine a Light-weight distribution package in Composer #543

Merged
merged 4 commits into from Jan 19, 2013

9 participants

@carlosbuenosvinos

In order to save space and bandwidth when installing doctrine using Composer, I have added .gitattributes removing files and folders unnecessary when using doctrine as a dependecy.

(extracted from http://getcomposer.org/doc/02-libraries.md#light-weight-distribution-packages)

Including the tests and other useless information like .travis.yml in distributed packages is not a good idea.

The .gitattributes file is a git specific file like .gitignore also living at the root directory of your library. It overrides local and global configuration (.git/config and ~/.gitconfig respectively) when present and tracked by git.

Use .gitattributes to prevent unwanted files from bloating the zip distribution packages.

// .gitattributes
/Tests export-ignore
phpunit.xml.dist export-ignore
Resources/doc/ export-ignore
.travis.yml export-ignore
Test it by inspecting the zip file generated manually:

git archive branchName --format zip -o file.zip

Note: Files would be still tracked by git just not included in the distribution. This will only work for GitHub packages installed from dist (i.e. tagged releases) for now.

@stof stof commented on an outdated diff Dec 28, 2012
.gitattributes
@@ -0,0 +1,16 @@
+/tests export-ignore
+/tools export-ignore
+.gitattribues export-ignore
+.gitignore export-ignore
+.gitmodules export-ignore
+.travis.yml export-ignore
+build.properties export-ignore
+build.properties.dev export-ignore
+build.xml export-ignore
+CONTRIBUTING.md export-ignore
+doctrine-mapping.xsd export-ignore
+LICENSE export-ignore
@stof
Doctrine member
stof added a note Dec 28, 2012

you should not ignore the license

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff Dec 28, 2012
.gitattributes
@@ -0,0 +1,16 @@
+/tests export-ignore
+/tools export-ignore
+.gitattribues export-ignore
+.gitignore export-ignore
+.gitmodules export-ignore
+.travis.yml export-ignore
+build.properties export-ignore
+build.properties.dev export-ignore
+build.xml export-ignore
+CONTRIBUTING.md export-ignore
+doctrine-mapping.xsd export-ignore
@stof
Doctrine member
stof added a note Dec 28, 2012

you should not ignore the XSD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof commented on an outdated diff Dec 28, 2012
.gitattributes
+/tests export-ignore
+/tools export-ignore
+.gitattribues export-ignore
+.gitignore export-ignore
+.gitmodules export-ignore
+.travis.yml export-ignore
+build.properties export-ignore
+build.properties.dev export-ignore
+build.xml export-ignore
+CONTRIBUTING.md export-ignore
+doctrine-mapping.xsd export-ignore
+LICENSE export-ignore
+phpunit.xml.dist export-ignore
+README.markdown export-ignore
+run-all.sh export-ignore
+UPGRADE.md export-ignore
@stof
Doctrine member
stof added a note Dec 28, 2012

The README and the UPGRADE files shouold be distributed. They are meant for users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosbuenosvinos

Ok, and for the rest?

@doctrinebot

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2215

@carlosbuenosvinos

I have added license, xsd and upgrade

Carlos Buenosvinos Update .gitattributes
Fix TYPO
1b5d431
@beberlei
Doctrine member

@carlosbuenosvinos can you add the README.md again? I will merge then.

@fprochazka

This is great! Will be added .gitattributes also to dbal & common?

@beberlei beberlei merged commit 3a4331d into doctrine:master Jan 19, 2013

1 check passed

Details default The Travis build passed
@Ocramius Ocramius added a commit to Ocramius/dbal that referenced this pull request Jan 19, 2013
@Ocramius Ocramius Lightweight export as of doctrine/doctrine2#543 e92fc15
@Ocramius Ocramius referenced this pull request in doctrine/dbal Jan 19, 2013
Merged

Lightweight export as of doctrine/doctrine2#543 #250

@Ocramius
Doctrine member
@igorw
igorw commented Sep 17, 2013

We decided to revert this for symfony, as having the docs and tests handy can be very useful. Stripping such stuff out for deployment is not part of composer's responsibility.

The reason I bring it up is because somebody asked about it in #composer.

@Ocramius
Doctrine member

@igorw it's not part of composer's responsibility, but still ok for a dist package (zip).

People using the ZIP aren't going to compile the RST docs anyway as far as I know

@igorw
igorw commented Sep 17, 2013

The may not want to compile them, but possibly still want to read them. For reference, here's the discussion we had for symfony: symfony/symfony#6605

@Ocramius
Doctrine member

@igorw heh, on the other side there's projects like drupal that basically want to reduce package size no matter what...

@m2mdas
m2mdas commented Sep 18, 2013

I agree with @igorw . When I started working in Symfony years ago these tests were invaluable for understanding usage. It is especially important for Dotcrine2 as docs in the site usually show example but the tests work as sample implementation. Besides that opening another browser tab for seeing the doc or cloning the repo just for reading tests feels a little cumbersome :)

@AdamWill

FWIW this is a shame for downstream distributions too. We like to run the unit tests at package build time to verify functionality; if the dist does not include the tests, we either have to tar them up ourselves and go to the trouble of updating that tarball every time the package gets bumped, or skip running the tests.

@Ocramius Ocramius self-assigned this Dec 30, 2014
@Ocramius
Doctrine member

This is not going to change: if you need packages with pre-dist state, then don't get a distribution package, get a source package.

@AdamWill

What 'source package' do you mean?

Putting the tests in .gitignore drops them from all github-generated tarballs, AFAICS. They're not in the 'Source code' tarballs from https://github.com/doctrine/dbal/releases or https://github.com/doctrine/dbal/tags . The only way we could get them is to spin our own tarball, one way or another, AFAICS.

@fprochazka

@AdamWill just remove the vendor/ directory completely, and when installing dependencies add --prefer-source option

composer install --no-interaction --prefer-source --dev

then you'll get the whole codebase even with tests for all dependencies

@AdamWill

@fprochazka thanks, but that's completely irrelevant for distribution packaging, we don't get the sources from composer.

@Ocramius
Doctrine member

You don't get anything from composer, only a link to the download or clone URL on github. Adding a --prefer-source as @fprochazka suggests is simply pointing your composer client at the GIT URI, which leads to having the sources.

@AdamWill

I'm a distribution packager. We do not use composer to retrieve the sources in any way. That's not at all appropriate for downstream distribution; we need a canonical tarball. We use the tarballs generated by github.

We can check out a git repo and build a tarball from it, but we prefer not to, because it's a manual step requiring documentation to reproduce, it reduces confidence in the reproducibility of the build process. It's much better if we have a valid canonical upstream URL to point to. This is the Fedora convention for github-sourced code:

%global github_owner    doctrine
%global github_name     dbal
%global github_commit   dd4d1062ccd5018ee7f2bb05a54258dc839d7b1e

Source0:       https://github.com/%{github_owner}/%{github_name}/archive/%{github_commit}/%{github_name}-%{github_commit}.tar.gz

It's reproducible and requires no manual work: so long as github exists and doesn't drop that URL format, that URL will give you the correct tarball. I package quite a few PHP-ish things from github, and doctrine-DBAL is the only one I've found so far which drops its tests from its github tarballs in this way.

@Ocramius
Doctrine member

@AdamWill yeah, and my suggestion is (since you are actually "building from source", as far as I can see) to drop that approach and use source packages instead.

Yes, it will take more effort for you, but we still don't need all those files in deployments, not to mention that stuff like example directories left lying around, as well as test assets that allow RCE (not in this project, but many others) are a security issue as well

@AdamWill

People downloading tarballs from github pretty clearly want a source distribution, I'd say. That's what the github download links say, remember? "Source code".

As you keep mentioning, people who just want to pull the module into their PHP project use composer, they don't grab tarballs from github. Is there not a mechanism by which you can exclude the files from composer distribution without excluding them from github distribution?

@Ocramius
Doctrine member

people who just want to pull the module into their PHP project use composer, they don't grab tarballs from github

composer actually simply forwards the downloads to one of the URLs that you mentioned above.

@fprochazka

Yes, source code of the lib. Not the tests.

No, composer uses github directly, so having it provide the tests in tarbals is wrong from composer point of view.

This question might be a bit off topic but... What is the point of including php libs as packages in linux distribution?

@AdamWill

same reason distributions include any libraries: so they can be reused between consumers and so you don't have sixty three copies of Doctrine to update whenever a security issue shows up. yes, this is all terribly old-school and not at all cool in the current climate where everyone's going bananas for single-purpose vertical stacks, but us cranky old distribution folks still worry about it. it is off-topic, yeah, so let's not get into it, I'm cranky enough this morning.

I see where you're coming from given the limitations of composer (oh, the limitations of composer, my new book coming soon...), but I'd still prefer to have the tests, and most other projects seem to do it.

@fprochazka

That's just because their users haven't yet pushed them into including .gitattributes. So it's just a matter of time. I wouldn't count on it working in the future.

@Ocramius
Doctrine member

Larger projects such as zf2 also tend to do that, since the test suite is really much larger than the codebase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.