Add install-path and type to installedVersions.php and installed.php,… #9699

ochorocho · 2021-02-15T17:08:43Z

… add method to get installed packages by type

Extended installedVersions.php and installed.php with type and install_path
Added getInstalledPackagesByType to allow retrieval of of installed packages by a given type

@Seldaek your feedbeack is very welcome :-)

ochorocho · 2021-02-16T13:05:47Z

Just had a look whats going on with phpstan issues.
Get 352 errors, even when running it on master

 [ERROR] Found 352 errors

src/Composer/Repository/FilesystemRepository.php

ochorocho · 2021-02-16T16:52:52Z

@stof can't wrap my head around this phpstan issue. It says Call to an undefined static method. The method is there, and it works locally and in "most" Actions except windows
Any idea what could cause this?

https://github.com/composer/composer/pull/9699/checks?check_run_id=1912250647

Seldaek · 2021-02-24T20:09:29Z

@ochorocho sorry for the trouble but could you rebase this on latest master? I merged other things which conflict with your PR.

As for the phpstan error, you can ignore these, it's a quirk of the fact the InstalledVersions file used in CI does not yet contain the new methods until the PR gets merged.

ochorocho · 2021-02-25T09:19:34Z

@Seldaek rebase done :-)

ochorocho · 2021-03-08T13:11:53Z

FYI, all packages defined with self.version are missing paths.
Working on a fix right now.

EDIT: Seems like a TYPO3 specific issue. Only relevant for development in the mono repo.

src/Composer/Repository/FilesystemRepository.php

src/Composer/InstalledVersions.php

Seldaek · 2021-03-16T13:31:31Z

FYI, all packages defined with self.version are missing paths.

I am not sure what you mean, if you have more details maybe we can figure out a solution or at least identify the root cause.

ochorocho · 2021-03-16T14:34:10Z

FYI, all packages defined with self.version are missing paths.

I am not sure what you mean, if you have more details maybe we can figure out a solution or at least identify the root cause.

install_path was missing for some packages in vendor/composer/InstalledVersions.php.
Checked it again after i applied your suggestion and it seems to work now.

example:

    'typo3/cms-dashboard' => 
    array (
      'dev-requirement' => false,
      'replaced' => 
      array (
        0 => '11.2.x-dev',
        1 => 'dev-master',
      ),
    ),

composer.json of the TYPO3 project https://git.typo3.org/Packages/TYPO3.CMS.git/blob_plain/HEAD:/composer.json
In case i see this appearing again i'll open an seperate issue.

Thank you.

Let me know if you need anything else. :-)

Seldaek · 2021-03-16T16:14:37Z

Thanks for the fixes. The last thing open is #9699 (review) which definitely needs fixing but then it looks good to me.

stof · 2021-03-16T17:38:40Z

tests/Composer/Test/InstalledVersionsTest.php

            'pretty_version' => 'dev-master',
            'version' => 'dev-master',
+            'type' => 'library',
+            'install_path' => false,


string|null is a better type than string|false IMO

Changed to null

ochorocho · 2021-03-17T12:29:17Z

Thanks for the fixes. The last thing open is #9699 (review) which definitely needs fixing but then it looks good to me.

@Seldaek now using an array containing package name and its path and passing it to generateInstalledVersions.
The root package path is now 'install_path' => '', and metapackage using 'install_path' => NULL,

stof · 2021-03-17T15:10:24Z

tests/Composer/Test/Repository/Fixtures/installed.php

        'pretty_version' => 'dev-master',
        'version' => 'dev-master',
+        'type' => 'library',
+        'install_path' => false,


this is wrong

Changed to 'install_path' => '',.

Thank you!

stof

I don't see any tests covering the generation of the install_path key

stof · 2021-03-18T10:48:08Z

src/Composer/InstalledVersions.php

+    /**
+     * Initialize $installed array
+     */
+    public static function __constructStatic() {}


why __constructStatic ? names prefixed by __ are soft-reserved by PHP for magic methods, and this is not one of them.

Makes sense, will rename it.

stof · 2021-03-18T10:48:39Z

src/Composer/InstalledVersions.php

    }
 }
+
+InstalledVersions::__constructStatic();


why ? this is a no-op

stof · 2021-03-18T10:51:09Z

src/Composer/Repository/FilesystemRepository.php

+            } else {
+                $stringContent = str_replace(array('\\', '\''), array('\\\\', '\\\''), $value);
+                $folder = $key === 'install_path' ? '__DIR__ . DIRECTORY_SEPARATOR . ' : '';
+                $lines .= $folder . '\'' . $stringContent . "',\n";


you should still be using var_export to export the string literal, rather than rebuilding it in userland (in an incomplete way)

Well, i need this to be able to write __DIR__ . DIRECTORY_SEPARATOR . to the PHP file and not the resulting path.
Happy to have any suggestions to avoid using my own method.

Your generated line is __DIR__ . DIRECTORY_SEPARATOR . followed by a string literal. I'm talking only about the string literal, not about the whole line.

stof · 2021-03-18T10:52:38Z

src/Composer/Repository/FilesystemRepository.php

                $reference = ($package->getSourceReference() ?: $package->getDistReference()) ?: null;
            }

+            $installPath = $package instanceof RootPackageInterface ? '' : $installPaths[$package->getName()];


this is wrong. the relative path from the location of the installed.php file to the root package is not the empty string.

This is the final result in InstalledVersions.php so i reckon the path correct:

self::$installed = array( 'root' => array( 'pretty_version' => 'dev-master', 'version' => 'dev-master', 'type' => 'library', 'install_path' => __DIR__ . DIRECTORY_SEPARATOR . '', 'aliases' => array( 0 => '2.1.x-dev', ), 'reference' => 'a614587ec91031b99ab23b385838f6d9641c847d', 'dev-requirement' => true, 'name' => 'composer/composer', ), ....

root should have an install_path of __DIR__ . '/../../' for composer, but the result should be dependent on the vendor-dir, so it should really compute the relative path from vendor-dir/composer to getcwd() to find the root install path.

stof · 2021-03-18T16:12:27Z

src/Composer/Repository/FilesystemRepository.php

+                $config = $package->getConfig();
+                $vendorDir = !empty($config['vendor-dir']) ? $config['vendor-dir'] : 'vendor';
+                $to = getcwd();
+                $from = $to . DIRECTORY_SEPARATOR . $vendorDir . DIRECTORY_SEPARATOR . 'composer';


no need to do your own processing of the vendor-dir config. The $from is the $repoDir that we have in ->write(), i.e. the folder in which we are generating the file. You only need to pass it as an argument in this private method

Thanks for that hint. Changed it.

ochorocho · 2021-03-18T17:15:21Z

@Seldaek tried to fix testRepositoryWritesInstalledPhp but failed. :-(
The path is confusing me /foo/bar/vendor/woop/woop.

Can you give me any advice to get this test running again?

ochorocho · 2021-04-01T08:35:52Z

@Seldaek all tests passed now. Branch is up to date with master.

… add method to get installed packages by type Issue composer#9648

…pdoc types

…ECTORY_SEPARATOR

…hich do not have a shortest-path and require an absolute path to be addressed

…ta from the class

Seldaek · 2021-05-21T12:48:56Z

@ochorocho thanks for this, I rebased it again and squashed commits to clean up a little then reviewed in depth and did some changes (some of it are kinda unrelated to this PR, just other things I noticed needed tweaking before 2.1 for InstalledVersions while I was poking around). Have a look at my individual commits if you're interested in more details.

I think this is mergeable now, but happy if someone else has a second to review again.

ochorocho

Looks good to me.

Thank you! :-)

ochorocho force-pushed the improve-installed-versions-9648 branch from c9d325b to 85fe2a9 Compare February 16, 2021 08:47

stof suggested changes Feb 16, 2021

View reviewed changes

src/Composer/Repository/FilesystemRepository.php Outdated Show resolved Hide resolved

Seldaek added this to the 2.1 milestone Feb 22, 2021

ochorocho force-pushed the improve-installed-versions-9648 branch from 08e200f to f4cbe78 Compare February 25, 2021 09:01

mindplay-dk mentioned this pull request Feb 27, 2021

Add support for stream-wrapper mindplay-dk/composer-locator#9

Open

bmack mentioned this pull request Mar 10, 2021

Feature/command search format json #9747

Merged

Seldaek mentioned this pull request Mar 11, 2021

[Feature] Command to dump files -> package mapping #9769

Closed

mvorisek reviewed Mar 16, 2021

View reviewed changes

src/Composer/Repository/FilesystemRepository.php Show resolved Hide resolved

Seldaek reviewed Mar 16, 2021

View reviewed changes

src/Composer/InstalledVersions.php Outdated Show resolved Hide resolved

Seldaek reviewed Mar 16, 2021

View reviewed changes

src/Composer/InstalledVersions.php Outdated Show resolved Hide resolved

stof reviewed Mar 16, 2021

View reviewed changes

ochorocho mentioned this pull request Mar 17, 2021

Replaced packages do not have a install_path #9784

Closed

stof reviewed Mar 17, 2021

View reviewed changes

stof reviewed Mar 18, 2021

View reviewed changes

ochorocho force-pushed the improve-installed-versions-9648 branch from fd6e18f to 6c9b55b Compare April 1, 2021 07:44

ochorocho mentioned this pull request Apr 23, 2021

Extend plugin to generate PackageStates.php and ActiveExtensions.php TYPO3/CmsComposerInstallers#121

Closed

ochorocho and others added 2 commits May 21, 2021 14:33

Add install-path and type to installedVersions.php and installed.php,…

b6c9d34

… add method to get installed packages by type Issue composer#9648

Rename dev-requirement to dev_requirement for consistency

8335b49

Seldaek added 5 commits May 21, 2021 14:33

Add InstalledVersions::getInstallPath($package) getter, and update ph…

5a69a1e

…pdoc types

Clean up dump code to avoid reimplementing var export, and remove DIR…

518b44a

…ECTORY_SEPARATOR

Fix handling of metapackages with null paths, and handling of paths w…

3fe4f84

…hich do not have a shortest-path and require an absolute path to be addressed

Undo addition of initializeInstalled and remove inline copy of the da…

82cffa1

…ta from the class

Document new methods and update the composer-runtime-api version to 2.1

284ec95

Seldaek force-pushed the improve-installed-versions-9648 branch from 7ea71f2 to 284ec95 Compare May 21, 2021 12:46

Fix type

019d051

ochorocho commented May 21, 2021

View reviewed changes

Seldaek merged commit da3d5e3 into composer:master May 24, 2021

Uh oh!

Add install-path and type to installedVersions.php and installed.php,… #9699

Add install-path and type to installedVersions.php and installed.php,… #9699

Uh oh!

Conversation

ochorocho commented Feb 15, 2021

Uh oh!

ochorocho commented Feb 16, 2021

Uh oh!

Uh oh!

ochorocho commented Feb 16, 2021

Uh oh!

Seldaek commented Feb 24, 2021

Uh oh!

ochorocho commented Feb 25, 2021

Uh oh!

ochorocho commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Seldaek commented Mar 16, 2021

Uh oh!

ochorocho commented Mar 16, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Seldaek commented Mar 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ochorocho commented Mar 17, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ochorocho Mar 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ochorocho commented Mar 18, 2021

Uh oh!

ochorocho commented Apr 1, 2021

Uh oh!

Seldaek commented May 21, 2021

Uh oh!

ochorocho left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

ochorocho commented Mar 8, 2021 •

edited

Loading

ochorocho commented Mar 16, 2021 •

edited

Loading

ochorocho Mar 18, 2021 •

edited

Loading