Failure marker file should be excluded using a relative path

Ok added a test. The test fails on 3.0.x

1. How is it possible that this was never noticed? 😱
2. How will we prevent this from reoccurring in the future? (No generic test coverage was added here, only test coverage for this one specific case.)

\PhpTuf\ComposerStager\Domain\Value\PathList\PathListInterface::add() has as a default implementation \PhpTuf\ComposerStager\Infrastructure\Value\PathList\PathList::add() which looks like this:

    public function add(array $paths): void
    {
        $this->assertValidInput($paths);
        $this->paths = array_merge($this->paths, $paths);
    }

which does:

    /**
     * @param array<string> $paths
     *
     * @throws \PhpTuf\ComposerStager\Domain\Exception\InvalidArgumentException
     */
    private function assertValidInput(array $paths): void
    {
        foreach ($paths as $path) {
            /** @psalm-suppress DocblockTypeContradiction */
            if (!is_string($path)) {
                $given = is_object($path)
                    ? $path::class
                    : gettype($path);

                throw new InvalidArgumentException(sprintf(
                    'Paths must be strings. Given %s.',
                    $given,
                ));
            }
        }
    }

So it just validates that it receives strings. Not relative paths. IOW: this needs an upstream bugfix in Composer Stager, because the input validation is inadequate.

Reopening for that.

… and we need a follow-up to either convert the test that was added here to a generic test, or to remove the test that was introduced here.

Yep sorry I meant make upstream bug report.

from chatting with @travis.carden the intend behavior for exclusions is

1. Paths can be relative or absolute
2. but absolute paths should only be used if they are within the source directory.

In our case on apply() the source directory is our stage directory so this should have always been relative.

Not sure why this worked before with php. The rsync copier acts as intended.

will make follow-ups

Upstream follow-up Exclude paths act differently between PHP and Rsync file copier

but absolute paths should only be used if they are within the source directory.

Makes sense. And that's indeed the upstream bug. Clarified that at https://github.com/php-tuf/composer-stager/issues/176#issuecomment-15733....

Closing since we have upstream issue

🥳

In which issue did Automatic Updates start requiring a Composer Stager version that includes this fix, which is what allowed us to close this?

We can leave this open for committing the requirement change when https://github.com/php-tuf/composer-stager/issues/176 is committed.

I think though we should consider why CollectPathsToExcludeEvent needs to implement PathListInterface.

PathListInterface only has to methods getAll() and add()

for add() we already have 2 methods on <code>CollectPathsToExcludeEvent , addPathsRelativeToWebRoot() and addPathsRelativeToProjectRoot() so we never call CollectPathsToExcludeEvent::add()

I suspect this is now a duplicate of 📌 FailureMarker should be responsible for excluding the marker file from stage operations Fixed .

This is a duplicate of a fixed issue, which had upstream aspects which were also corrected months ago. I think this is thoroughly done.