phar-composer
phar-composer copied to clipboard
Avoid dev packages when building the phar
I'm not sure what "version" was being referred to when choosing installed.json
over composer.lock
, but the former has no notion of dev/no-dev whereas the later does. This works for me, but is a breaking change in that the current default includes dev packages. Alternatively there could be --dev
/ --no-dev
options to build
defaulting to --dev
, but allowing --no-dev
similar to composer install
itself.
I'm not sure what "version" was being referred to when choosing
installed.json
overcomposer.lock
Neither am I ;-) IIRC this was chosen in order to check which packages have actually been installed, vs which one have been locked.
IMHO it makes sense to skip pure require-dev
packages either way.
As such, I'm all for getting this in! Can you update your PR with a few test cases to confirm this doesn't break anything?
I'm all for getting this in! Can you update your PR with a few test cases to confirm this doesn't break anything?
Ping @radford, is there anything I can help you with? :)
Upon further inspection I think we should consider a few more things (just listing some random thoughts):
- Running
composer install --no-dev
and then running phar-composer returns the expected phar with no dev dependencies installed (arguably a bit cumbersome, but explicit) - Running
composer install
(the default) and then running phar-composer returns a phar with dev dependencies installed (arguably not expected, but in line with how composer works) - The
composer.lock
is identical in both cases, whereas theinstalled.json
only lists packages actually installed - Running
phar-composer install
already runscomposer install --no-dev
- If we install with dev dependencies but only pick non-dev dependencies, then the autoloader still contains dev dependencies (unlikely, but possible, to cause issues)
- What if we run in a bare repository with only a
composer.json
(often the case with libraries)? - What if we run in a bare repository with both a
composer.json
andcomposer.lock
(often the case with application projects)?
As an alternative approach, we may also look into merely detecting if dev dependencies have been installed. We could then either ask the user to confirm or abort or in the future also suggest installing into a temporary directory without dev dependencies.
Any thoughts?
Do you have a use case where dev dependencies in a phar make sense? To my mind, if you are releasing your dev dependencies, then they are not dev dependencies. I'd would suggest bumping the major version and dropping them.
Do you have a use case where dev dependencies in a phar make sense?
Nope, afaict it never makes sense to bundle the dev dependencies and I'm all for dropping them! :+1:
My previous comment was more about how we should drop them and what we have to keep in mind. Afaict some of these aren't addressed currently, so I'm not sure we should merge this yet.
What are your thoughts on this?
If we install with dev dependencies but only pick non-dev dependencies, then the autoloader still contains dev dependencies (unlikely, but possible, to cause issues)
This is a wart for sure, but I agree it is unlikely to cause problems.
What if we run in a bare repository with only a
composer.json
(often the case with libraries)?
I don't see the point of this use case. If the library has dependencies, then it should have a composer.lock
to fix those versions.
What if we run in a bare repository with only a composer.json (often the case with libraries)? What if we run in a bare repository with both a composer.json and composer.lock
If you are referring to a git bare repository, then I don't see how it makes sense to run in a bare repository. If by bare you mean in a repository where composer install
hasn't been run, then I don't have an opinion.
Overall I think dropping dev dependencies is bug fix, but we should bump the version number in case there is someone that depends on the old behavior.
I suggest making the documented way to use phar-composer
itself be as a dev dependency (which of course you wouldn't want to install) of any package that uses it for releases with its version number locked like as with your other dependencies.
" If the library has dependencies, then it should have a composer.lock to fix those versions." - seriously? since when libraries have composer.lock? Libraries enforce "versions" with upper/lower bounds of versions in composer.json, and not the .lock.
Thanks for filing this PR @Grisou13, I'd love to see this in! :shipit: Unfortunately, this PR is currently incomplete and hasn't seen any updates in some time.
I've just moved most of the discussion to #61, perhaps we should address that one first before continuing this PR? :+1:
Would you guys care to comment on this ticket first? Thanks!
@clue, could you be more specific about what's missing from the PR? I stand by my contention that this is bug fix and the incompatibility should be handled by bumping the version number.
I'd like to discuss this feature in #61, so that this PR only has to deal with its implementation details :+1:
In particular two things are worthy of discussion (in #61):
- What about the autoloader, because it contains entries for
require-dev
? - Should this be considered a BC break?
Do you have a use case where dev dependencies in a phar make sense?
Personally, I'm fine with dropping them. However, if one needs to release a dev
and a no-dev
version, things get obviously complicated. E.g.:
# lean production version
$ php my.phar serve
# development version
$ php my_dev.phar serve --debug
$ php my_dev.phar run-tests # internal unit tests on the current platform
$ php my_dev.phar create-docs docs/ # create html docs
Any news on this ?