Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

[monorepo-builder] MBConfig::packageDirectoriesExcludes input validation makes exclusion impossible #4389

Open
Kingdutch opened this issue Sep 6, 2022 · 2 comments

Comments

@Kingdutch
Copy link

Problem

MBConfig::packageDirectoriesExcludes performs an Assert::allFileExists($packageDirectories);. This means that paths must either be absolute or relative to the current repository root.

However, Symfony's ExcludeDirectoryFilterIterator will turn any non-simple (i.e. more than one path segment) path into a matcher. The actual matching then happens against a path that is relative to the directories provided to packageDirectories. That's usually already a step removed from the repository root.

This means that the input that is valid for packageDirectoriesExcludes will never match in ExcludeDirectoryFilterIterator.

An example folder structure

  tools/
    oste/
      data/
        composer.json
      composer.json

Desired: Get all packages in tools/*/ but exclude specifically tools/oste/data/composer.json.

Config that passes input validation but doesn't match anything:

  $mbConfig->packageDirectories([
    __DIR__ . '/tools',
  ]);

  $mbConfig->packageDirectoriesExcludes([
    'tools/oste/data',
  ]);

When debugging the matching in ExcludeDirectoryFitlerIterator you will see that oste and oste/data are matched against the exclude input which is tools/oste/data which never matches.

Config that would match the exclude but doesn't pass input validation:

  $mbConfig->packageDirectories([
    __DIR__ . '/tools',
  ]);

  $mbConfig->packageDirectoriesExcludes([
    'oste/data',
  ]);

This doesn't pass input validation because MBConfig::packageDirectoriesExcludes tries to check that oste/data exists (which it doesn't because it lives in tools/oste/data).

Proposed solution

Either modify MBConfig::packageDirectoriesExcludes to remove the folder existence validation. This allows excluding subdirectory patterns in included directories. A downside is that I may want to include directoreis A/ and B/ and exclude B/Foo but not A/Foo which would then not be possible (since the iterator matches only on subpaths).

An alternative is to make sure that ExcludeDirectoryFilterIterator actually gets the path relative to the project root to match on, rather than relative to the include directory. That would allow configuring an exclusion of B/Foo specifically.

Workaround

The deprecated $mbConfig->parameters()->set(Option::PACKAGE_DIRECTORIES_EXCLUDES, []); can be used to work around the faulty input validation in MBConfig.

@TomasVotruba
Copy link
Member

Thanks for sharing the issue in detail.

Could you kick off with a failing PR for your situation? It would be easier to iterate from there.

@Kingdutch
Copy link
Author

@TomasVotruba Here you go: #4390 :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants