search

nextcloud / photos

📸 Your memories under your control
GNU Affero General Public License v3.0
590 stars 62 forks source link

Dev dependencies pollute global namespace #1919

Closed ChristophWurst closed 3 months ago

ChristophWurst commented 1 year ago

Describe the bug

https://docs.nextcloud.com/server/latest/developer_manual/app_development/dependency_management.html#development-tools

Dev dependencies are required in the app's root composer file, so they do get autoloader: https://github.com/nextcloud/photos/blob/0c9284239fd4f5f178bc88639d9300d3f9ca9b70/composer.json#L36. Every dependency of vimeo/psalm can conflict with server dependencies and dependencies of other apps.

To Reproduce Steps to reproduce the behavior:

  1. Install composer dependencies on my dev env

Expected behavior

Clean environment

Screenshots If applicable, add screenshots to help explain your problem.

Desktop (please complete the following information):

Smartphone (please complete the following information):

Browser log

Open your console, reload your page and/or do the action leading to this issue and copy/paste the log in this thread.
How to access your browser console (Click to expand) # Chrome - Press either CTRL + SHIFT + J to open the “console” tab of the Developer Tools. - Alternative method: 1. Press either CTRL + SHIFT + I or F12 to open the Developer Tools. 2. Click the “console” tab. # Safari - Press CMD + ALT + I to open the Web Inspector. - See Chrome’s step 2. (Chrome and Safari have pretty much identical dev tools.) # IE9 1. Press F12 to open the developer tools. 2. Click the “console” tab. # Firefox - Press CTRL + SHIFT + K to open the Web console (COMMAND + SHIFT + K on Macs). - or, if Firebug is installed (recommended): 1. Press F12 to open Firebug. 2. Click on the “console” tab. # Opera 1. Press CTRL + SHIFT + I to open Dragonfly. 2. Click on the “console” tab.

Additional context

A reference PR can be seen at https://github.com/nextcloud/twofactor_admin/pull/290.

ChristophWurst commented 1 year ago

When I install deps I see

  - Installing bamarni/composer-bin-plugin (1.8.2): Extracting archive
  - Installing composer/package-versions-deprecated (1.11.99.5): Extracting archive
  - Installing composer/pcre (3.1.0): Extracting archive
  - Installing psr/cache (3.0.0): Extracting archive
  - Installing doctrine/deprecations (v1.0.0): Extracting archive
  - Installing doctrine/lexer (2.1.0): Extracting archive
  - Installing doctrine/annotations (2.0.1): Extracting archive
  - Installing hexogen/kdtree (v0.2.5): Extracting archive
  - Installing symfony/deprecation-contracts (v2.5.2): Extracting archive
  - Installing psr/container (1.1.2): Extracting archive
  - Installing symfony/service-contracts (v2.5.2): Extracting archive
  - Installing symfony/stopwatch (v5.4.19): Extracting archive
  - Installing symfony/polyfill-php80 (v1.27.0): Extracting archive
  - Installing symfony/process (v5.4.19): Extracting archive
  - Installing symfony/polyfill-php81 (v1.27.0): Extracting archive
  - Installing symfony/polyfill-mbstring (v1.27.0): Extracting archive
  - Installing symfony/polyfill-php73 (v1.27.0): Extracting archive
  - Installing symfony/options-resolver (v5.4.19): Extracting archive
  - Installing symfony/finder (v5.4.19): Extracting archive
  - Installing symfony/polyfill-ctype (v1.27.0): Extracting archive
  - Installing symfony/filesystem (v5.4.19): Extracting archive
  - Installing psr/event-dispatcher (1.0.0): Extracting archive
  - Installing symfony/event-dispatcher-contracts (v2.5.2): Extracting archive
  - Installing symfony/event-dispatcher (v5.4.19): Extracting archive
  - Installing symfony/polyfill-intl-normalizer (v1.27.0): Extracting archive
  - Installing symfony/polyfill-intl-grapheme (v1.27.0): Extracting archive
  - Installing symfony/string (v5.4.19): Extracting archive
  - Installing symfony/console (v5.4.19): Extracting archive
  - Installing sebastian/diff (4.0.5): Extracting archive
  - Installing psr/log (1.1.4): Extracting archive
  - Installing composer/xdebug-handler (3.0.3): Extracting archive
  - Installing composer/semver (3.3.2): Extracting archive
  - Installing friendsofphp/php-cs-fixer (v3.14.4): Extracting archive
  - Installing nextcloud/coding-standard (v1.0.0): Extracting archive
  - Installing psr/clock (1.0.0): Extracting archive
  - Installing nextcloud/ocp (dev-master 6ec2f44): Extracting archive
  - Installing webmozart/assert (1.11.0): Extracting archive
  - Installing phpdocumentor/reflection-common (2.2.0): Extracting archive
  - Installing phpdocumentor/type-resolver (1.6.2): Extracting archive
  - Installing phpdocumentor/reflection-docblock (5.3.0): Extracting archive
  - Installing sebastian/version (3.0.2): Extracting archive
  - Installing sebastian/type (3.2.1): Extracting archive
  - Installing sebastian/resource-operations (3.0.3): Extracting archive
  - Installing sebastian/recursion-context (4.0.5): Extracting archive
  - Installing sebastian/object-reflector (2.0.4): Extracting archive
  - Installing sebastian/object-enumerator (4.0.4): Extracting archive
  - Installing sebastian/global-state (5.0.5): Extracting archive
  - Installing sebastian/exporter (4.0.5): Extracting archive
  - Installing sebastian/environment (5.1.5): Extracting archive
  - Installing sebastian/comparator (4.0.8): Extracting archive
  - Installing sebastian/code-unit (1.0.8): Extracting archive
  - Installing sebastian/cli-parser (1.0.1): Extracting archive
  - Installing phpunit/php-timer (5.0.3): Extracting archive
  - Installing phpunit/php-text-template (2.0.4): Extracting archive
  - Installing phpunit/php-invoker (3.1.1): Extracting archive
  - Installing phpunit/php-file-iterator (3.0.6): Extracting archive
  - Installing theseer/tokenizer (1.2.1): Extracting archive
  - Installing nikic/php-parser (v4.15.4): Extracting archive
  - Installing sebastian/lines-of-code (1.0.3): Extracting archive
  - Installing sebastian/complexity (2.0.2): Extracting archive
  - Installing sebastian/code-unit-reverse-lookup (2.0.3): Extracting archive
  - Installing phpunit/php-code-coverage (9.2.26): Extracting archive
  - Installing phar-io/version (3.2.1): Extracting archive
  - Installing phar-io/manifest (2.0.3): Extracting archive
  - Installing myclabs/deep-copy (1.11.1): Extracting archive
  - Installing doctrine/instantiator (1.5.0): Extracting archive
  - Installing phpunit/phpunit (9.6.8): Extracting archive
  - Installing sabre/uri (2.3.2): Extracting archive
  - Installing sabre/xml (2.2.5): Extracting archive
  - Installing sabre/vobject (4.5.3): Extracting archive
  - Installing sabre/event (5.1.4): Extracting archive
  - Installing sabre/http (5.1.6): Extracting archive
  - Installing sabre/dav (4.4.0): Extracting archive
  - Installing webmozart/path-util (2.3.0): Extracting archive
  - Installing openlss/lib-array2xml (1.0.0): Extracting archive
  - Installing netresearch/jsonmapper (v4.1.0): Extracting archive
  - Installing felixfbecker/language-server-protocol (v1.5.2): Extracting archive
  - Installing felixfbecker/advanced-json-rpc (v3.2.1): Extracting archive
  - Installing dnoegel/php-xdg-base-dir (v0.1.1): Extracting archive
  - Installing amphp/amp (v2.6.2): Extracting archive
  - Installing amphp/byte-stream (v1.8.1): Extracting archive
  - Installing vimeo/psalm (4.30.0): Extracting archive

specifically symfony/console, sabre/ and psr/ are known to cause issues on dev envs

come-nc commented 3 months ago

Which branch? master uses psalm/phar already since https://github.com/nextcloud/photos/pull/2189 .

Regarding symfony/console and sabre/dav, not sure how to avoid pulling them to get correct psalm validation?

ChristophWurst commented 3 months ago

Not sure which version. I saw it yesterday with 27.1.11

occ circles:manage:list
An unhandled exception has been thrown:
Error: Call to undefined method Symfony\Component\Console\Output\ConsoleOutput::section() in /var/www/html/nextcloud/web/apps/circles/lib/Command/CirclesList.php:241
Stack trace:
#0 /var/www/html/nextcloud/web/apps/circles/lib/Command/CirclesList.php(230): OCA\Circles\Command\CirclesList->displayCircles()
#1 /var/www/html/nextcloud/web/apps/photos/vendor/symfony/console/Command/Command.php(255): OCA\Circles\Command\CirclesList->execute()
#2 /var/www/html/nextcloud/web/core/Command/Base.php(177): Symfony\Component\Console\Command\Command->run()
#3 /var/www/html/nextcloud/web/apps/photos/vendor/symfony/console/Application.php(992): OC\Core\Command\Base->run()
#4 /var/www/html/nextcloud/web/apps/photos/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand()
#5 /var/www/html/nextcloud/web/apps/photos/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun()
#6 /var/www/html/nextcloud/web/lib/private/Console/Application.php(218): Symfony\Component\Console\Application->run()
#7 /var/www/html/nextcloud/web/console.php(100): OC\Console\Application->run()
#8 /var/www/html/nextcloud/web/occ(11): require_once('/var/www/html/n...')

Regarding symfony/console and sabre/dav, not sure how to avoid pulling them to get correct psalm validation?

Composer bin for Psalm and pulling the deps there? That would make them autoload for the psalm binary but not for anything else.

ChristophWurst commented 3 months ago

Pulling your own copy of symfony/* for static analysis is a bit of a false guarantee anyway, unless you have a mechanism in place that keeps your symfony packages in lock with server's 3rdparty.