Libraries installed by composer should be in vendor directory #28

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
6 participants

enumag commented Mar 15, 2013

No description provided.

@enumag enumag referenced this pull request in nette/nette Mar 15, 2013

@dg dg Revert "renamed 'vendor' to 'tools'"
I hate this word, but it become a standard :-/

This reverts commit 65d4f85.
769e172
Contributor

fprochazka commented Mar 15, 2013

👍

Contributor

Vrtak-CZ commented Mar 15, 2013

👍

Member

milo commented Mar 18, 2013

👍

Owner

dg commented Apr 7, 2013

Is it really needed to have two folders for libraries? I understand some libs are not maintained by composer, but what about subfolder libs/other?

enumag commented Apr 7, 2013

This solution is much more intuitive then subdirectories. The main reason is clear autoloading - RobotLoader should never be used for autloading of composer installed libraries. I've already seen several people confused by this, they didn't add autoloading directives into composer.json of their addon "because RobotLoader will take care of it".

Contributor

fprochazka commented Apr 7, 2013

Every time I create project from sandbox, I have to fix the vendor-dir in composer.json or remove this line

->addDirectory(__DIR__ . '/../libs')

There is still a lot of libraries, that are not available via composer (and older projects). This is a solution.

Owner

dg commented Apr 7, 2013

Clear autoloading is something that users doesn't care about. It is done automatically. But two folders are not intuitive, specially for users which do not know composer.

hrach commented Apr 7, 2013

Everyone who uses composer need this.

Owner

dg commented Apr 7, 2013

I am using Composer and I do not need two directories.

Contributor

fprochazka commented Apr 7, 2013

Me neither, but I do want vendor/ to be autoloaded via composer, not robot loader. Thats all.

Owner

dg commented Apr 7, 2013

@hosiplan I thought we were talking about #28 (comment) ;-)

Contributor

fprochazka commented Apr 7, 2013

:) Currently, I don't think that composer would delete existing directories, that he's not managing. That's one thing that could change.

Another problem is gitignore. Almost everyone (and everyone should do it this way) is ignoring vendor/. When you have dependency, that is not installed via composer, you do not wan't it ignored.

Therefore having libs/ for hardcopied deps (commited to repo), and vendor/ for Composer makes sense to me.

Member

milo commented Apr 7, 2013

If vendor would not be the standard way, only libs looks great:

libs\
libs\OtherLibs
libs\by-composer

and + RobotLoader ignore rules.

But two folders are not intuitive, specially for users which do not know composer.

Well, when somebody use composer.phar update, they should known a little bit about composer.

enumag commented Apr 7, 2013

Those who don't know composer won't know the purpose of the vendor directory either which means they most probably woudn't use it but rather delete it.

Owner

dg commented Apr 7, 2013

@enumag If somebody deletes it, he brokes require 'vendor/autoload.php'; in bootstrap.

@milo Eh? When somebody use composer, it is not user who do not know composer. Obviously.

enumag commented Apr 7, 2013

Well in my opinion there shouldn't be any vendor/autoload.php file in the sandbox itself. You probably don't want the user to see something like "Required vendor/autoload.php not found" but we can check it's existence via file_exists in bootstrap and do something if it doesn't exist (error massage with "use composer install" or an attempt to load Nette from libs directory).

Owner

dg commented Apr 7, 2013

@enumag Majority of users do not know Composer.

enumag commented Apr 7, 2013

@dg: And? We can attempt to load vendor/autoload.php, if that does not exist, try libs/Nette/loader.php and if even that does not exist throw some error message with advice to either use composer install or copy Nette to libs directory.

Owner

dg commented Apr 7, 2013

Nice. This is exactly what sandbox does. Discussion is about "why have two folders libs & vendor".

enumag commented Apr 7, 2013

Which was already explained. Your only problem is the existence of two directories instead of one with subdirectories, right? If so, it's probably better to stick with the standard.

Member

milo commented Apr 7, 2013

@dg Sorry, I was thinking that vendor dir does not exists until user runs composer. I forgot that commit adds this dir permanently.

enumag commented Apr 7, 2013

Also with the new Nette addons portal being a composer packages repository I think it's a good idea to propagate composer installation to anyone who does not know composer - even if it means one "unnecessary" directory in sandbox.

Member

milo commented Apr 7, 2013

When you decide to stop use composer one small disadvantage of current state is, that you must create autoload.php because old one is rewritten.

Another solution can be drop current autoload.php and something like:

# bootstrap.php (not so pretty)
if (is_file(__DIR__ . '/../libs/vendor/autoload.php')) {
    require __DIR__ . '/../libs/vendor/autoload.php';
} elseif (is_file(__DIR__ . '/../libs/Nette/loader.php')) {
    require __DIR__ . '/../libs/Nette/loader.php';
} else {
    die("Nette Framework is expected in directory '" . realpath(__DIR__ . '/..') . "/libs/Nette' but not found. Check if the path is correct or edit file '" . __FILE__ . "'.");
}

$robotLoader->ignoreDirs .= ', ' . __DIR__ . '/../libs/vendor'; // or netterobots.txt
# composer.json
"config": {
    "vendor-dir": "libs/vendor"
},
# .gitignore
libs/vendor

EDIT:
I was looking for option, if composer allows to specify path to autoload.php, but it does not. It is hardcoded.

@ghost

ghost commented Jul 24, 2013

Lock? This is done 02820d3

dg closed this Jul 24, 2013

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