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

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

Merged
merged 4 commits into from
Jan 19, 2013
Merged

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

merged 4 commits into from
Jan 19, 2013

Conversation

carlosbuenosvinos
Copy link
Contributor

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.

build.xml export-ignore
CONTRIBUTING.md export-ignore
doctrine-mapping.xsd export-ignore
LICENSE export-ignore
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should not ignore the license

@carlosbuenosvinos
Copy link
Contributor Author

Ok, and for the rest?

@doctrinebot
Copy link

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
Copy link
Contributor Author

I have added license, xsd and upgrade

@beberlei
Copy link
Member

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

@fprochazka
Copy link
Contributor

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

beberlei added a commit that referenced this pull request Jan 19, 2013
Make doctrine a Light-weight distribution package in Composer
@beberlei beberlei merged commit 3a4331d into doctrine:master Jan 19, 2013
Ocramius added a commit to Ocramius/dbal that referenced this pull request Jan 19, 2013
@Ocramius
Copy link
Member

@hosiplan see doctrine/dbal#250

beberlei added a commit to doctrine/dbal that referenced this pull request Jan 19, 2013
@igorw
Copy link
Contributor

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
Copy link
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
Copy link
Contributor

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
Copy link
Member

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

@m2mdas
Copy link

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
Copy link

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
Copy link
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
Copy link

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
Copy link
Contributor

@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
Copy link

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

@Ocramius
Copy link
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
Copy link

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
Copy link
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
Copy link

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
Copy link
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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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
Copy link
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants