Skip to content

Commit

Permalink
Working on re-doing package handling.
Browse files Browse the repository at this point in the history
  • Loading branch information
taylorotwell committed Dec 5, 2014
1 parent 1b9562f commit 3a0afc2
Showing 1 changed file with 0 additions and 95 deletions.
95 changes: 0 additions & 95 deletions src/Illuminate/Support/ServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,90 +36,6 @@ public function __construct($app)
*/
abstract public function register();

/**
* Register the package's component namespaces.
*
* @param string $package
* @param string $namespace
* @param string $path
* @return void
*/
public function package($package, $namespace = null, $path = null)
{
$namespace = $this->getPackageNamespace($package, $namespace);

// In this method we will register the configuration package for the package
// so that the configuration options cleanly cascade into the application
// folder to make the developers lives much easier in maintaining them.
$path = $path ?: $this->guessPackagePath();

$config = $path.'/config';

if ($this->app['files']->isDirectory($config))
{
$this->app['config']->package($package, $config, $namespace);
}

// Next we will check for any "language" components. If language files exist
// we will register them with this given package's namespace so that they
// may be accessed using the translation facilities of the application.
$lang = $path.'/lang';

if ($this->app['files']->isDirectory($lang))
{
$this->app['translator']->addNamespace($namespace, $lang);
}

// Next, we will see if the application view folder contains a folder for the
// package and namespace. If it does, we'll give that folder precedence on
// the loader list for the views so the package views can be overridden.
$appView = $this->getAppViewPath($package);

if ($this->app['files']->isDirectory($appView))
{
$this->app['view']->addNamespace($namespace, $appView);
}

// Finally we will register the view namespace so that we can access each of
// the views available in this package. We use a standard convention when
// registering the paths to every package's views and other components.
$view = $path.'/views';

if ($this->app['files']->isDirectory($view))
{
$this->app['view']->addNamespace($namespace, $view);
}
}

/**
* Guess the package path for the provider.
*
* @return string
*/
public function guessPackagePath()
{
$path = (new ReflectionClass($this))->getFileName();

return realpath(dirname($path).'/../../');
}

/**
* Determine the namespace for a package.
*
* @param string $package
* @param string $namespace
* @return string
*/
protected function getPackageNamespace($package, $namespace)
{
if (is_null($namespace))
{
list($vendor, $namespace) = explode('/', $package);
}

return $namespace;
}

/**
* Register the package's custom Artisan commands.
*
Expand All @@ -141,17 +57,6 @@ public function commands($commands)
});
}

/**
* Get the application package view path.
*
* @param string $package
* @return string
*/
protected function getAppViewPath($package)
{
return $this->app['path.base']."/resources/views/packages/{$package}";
}

/**
* Get the services provided by the provider.
*
Expand Down

43 comments on commit 3a0afc2

@ibrasho
Copy link
Contributor

@ibrasho ibrasho commented on 3a0afc2 Dec 6, 2014

Choose a reason for hiding this comment

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

I'm so excited to see the new way of doing packages.

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

I have two packages that broke down with this commit:

https://github.com/dimsav/laravel-translatable
https://github.com/JeffreyWay/Laravel-4-Generators

Hope we get e new way of do it soon!

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

@jonathanpmartins Just about every laravel pacakge in existance was broken by this. 20 of mine were broken...

@woodja
Copy link

@woodja woodja commented on 3a0afc2 Dec 6, 2014

Choose a reason for hiding this comment

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

Ya, 6 of the packages I was using broke as well. Temporarily either removed or hacked them to get things working, but would like to see a fix (either instructions for changing the package initialization/configuration so I can fork and fix or adding a deprecated version of these methods back until package maintainers get a chance to change against specs).

@taylorotwell
Copy link
Member Author

@taylorotwell taylorotwell commented on 3a0afc2 Dec 6, 2014 via email

Choose a reason for hiding this comment

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

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

👍 What Taylor just said.

@woodja
Copy link

@woodja woodja commented on 3a0afc2 Dec 6, 2014

Choose a reason for hiding this comment

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

Yep, understood @taylorotwell. Targeting a new application for L5 and know this can happen. Keep up the good work. 👍

@rawaludin
Copy link

Choose a reason for hiding this comment

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

Nice word @taylorotwell

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

Taylor, thanks for work. Can you tell when will be beta-release ?

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@RomainLanz
Copy link

Choose a reason for hiding this comment

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

👍

@sebastiansulinski
Copy link
Contributor

Choose a reason for hiding this comment

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

On that note - @taylorotwell - any indication when Laravel 5 will go to stable release?
I've recently started learning Laravel and want to start building the new system using it, but not sure whether to wait for version 5 or start with 4 and then move things across. If you could give me some rough idea that would be fab.

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

any indication when Laravel 5 will go to stable release?

Second half of January 2015.

@sebastiansulinski
Copy link
Contributor

Choose a reason for hiding this comment

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

Great - many thanks @GrahamCampbell !

@rogue780
Copy link

Choose a reason for hiding this comment

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

@taylorotwell unfortunately some of us started working on projects with laravel 5 back when it was supposed to be released in November and have invested too much to back-port it to 4.2. I might have to at this point, though. We were going to be relying on laravel cashier and as I was plugging it in tonight I found this change to have broken it.

I'm not mad and I appreciate everything you're doing. I'm just pointing out that there was a time when the expectation for the release timeline was different.

@kaidesu
Copy link

Choose a reason for hiding this comment

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

@rogue780 Revert to the commit before this change to continue working with packages; though obviously won't be getting any updates moving forward until L5 is either fully released or gets the re-worked package handling pushed.

"require": {
        "laravel/framework": "dev-master#846c935194a036901ba6b4397c8897fa51e19111"
}

@rogue780
Copy link

Choose a reason for hiding this comment

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

@kaidesu because i'd have to roll back laravel/laravel to a similar point and that would cause too much refactoring at this point. It might be just as much as moving stuff to 4.2 tbh.

Again, I'm not complaining and I'm not blaming anyone but myself. I took a risk and it didn't pay off the way I wanted it too. I was just trying to articulate why some might get frustrated by this change since the timeline got pushed back.

As for me, I'm probably just going to use stripe/stripe-php directly instead of laravel/cashier and press on. I'm relying too much on route and event annotations as well as elixir at this point and don't want to change up my workflow.

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@otherguy
Copy link

Choose a reason for hiding this comment

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

Any idea on just how much package handling is going to change? I don't have any 3rd party packages in my Laravel 5 test project, but I'm writing a new package. If there will be too many changes from the way packages were created in 4.2, I'm going to wait for a stable(ish) release.

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

@darkwinternight i made migration of big package from 4.2 to 5.0 and it was very very easy - about 1 day of work

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

@pilot911 Can you explain more details ?

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

It's possible to migrate without actually changing much, but it doesn't mean you should. I'd recommend you change over to use the new contracts where possible for example, and embrace the new stuff 5.0 brings.

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

@jonathanpmartins just updated softDelete in model with trait from 5.0, replaced \View::make()->render() with view()->render() and updated custom user class with new field - that's all 👍 but you can replace some router's filter with middlewares like csrf - that's all very simple... but for developing package you must use only this commit:

"require": {
        "laravel/framework": "dev-master#846c935194a036901ba6b4397c8897fa51e19111"
}

@otherguy
Copy link

Choose a reason for hiding this comment

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

You do realize that this is exactly what we are trying to avoid, yes?

The changes you speak of are not really required for L5 (i.e., the View facade still exists).

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

@darkwinternight i know, but why don't use new features ? we use them and i wrote answer about our practice

does anybody know Taylor's paypal ?

@otherguy
Copy link

Choose a reason for hiding this comment

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

view() is merely a helper, you're still depending on the framework. Instead, you should use the new L5 contracts:

public function foo( Illuminate\Contracts\View\View $view ) {
}

But I digress. The point of this discussion is that it is attached to the commit 3a0afc2, which removes (among others) the package() method. Essentially you're saying we should use an older commit so as not to break anything (which by the way I was suggesting 2 weeks ago). That is not very helpful.

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

@darkwinternight you're right

@atrauzzi
Copy link
Contributor

Choose a reason for hiding this comment

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

Excited to see the new package conventions for 5! Don't leave it too long :)

@calvinchoy
Copy link

Choose a reason for hiding this comment

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

I reverted back to laravel/framework": "dev-master#846c935194a036901ba6b4397c8897fa51e19111 but the env method in this commit is not found. It works if I just the $_ENV variable instead though.

Anyone know how to get env() working on this commit?

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

nops, @calvinchoy I think the env() function enter after it.

@calvinchoy
Copy link

Choose a reason for hiding this comment

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

Ah ok, well guess it will be a temporary tradeoff. Use the old commit to keep necessary packages working and manually change env to $_ENV.

Hope it won't be too much work to port to the final version from this this commit =/

@ibrasho
Copy link
Contributor

@ibrasho ibrasho commented on 3a0afc2 Jan 9, 2015

Choose a reason for hiding this comment

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

I think it's clear now that there is no default way to handle packages in Laravel 5. Handle your package the way you like.

So no need to refer to the 846c935 commit, just update to the latest and try to get your package working using a ServiceProvider.

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

You actually need to do this in the register method, not the boot method otherwise you won't be able to customise the config in your app...

@pilot911
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks, but can you explain ?

@morrislaptop
Copy link
Contributor

Choose a reason for hiding this comment

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

I just put up an article with a possible workaround to this problem - https://medium.com/@morrislaptop/bridging-laravel-4-packages-to-laravel-5-2081f19643c6

UPDATE: I've made this a package which can be easily dropped in - https://github.com/morrislaptop/LaravelFivePackageBridges

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

A much better solution would be for everyone to use https://github.com/orchestral/config. I've moved all my laravel 5 packages over to it.

@jonathanpmartins
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@arce701
Copy link

Choose a reason for hiding this comment

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

How to solve the problem: BadMethodCallException in ServiceProvider.php line 226:
Call to undefined method [package].
The package is laravel-jqgrid

@onbjerg
Copy link

Choose a reason for hiding this comment

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

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

How to solve the problem

It's not a problem as such. It means the package you're trying to use is for Laravel 4 instead of Laravel 5.

@GrahamCampbell
Copy link
Member

Choose a reason for hiding this comment

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

Likely the maintainer set the wrong version constraints.

@carcinocron
Copy link

Choose a reason for hiding this comment

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

How to solve the problem

It's not a problem as such. It means the package you're trying to use is for Laravel 4 instead of Laravel 5.

How would a package maintainer upgrade a Laravel 4 package to Laravel 5? I'm still looking for that.

Please sign in to comment.