Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
Pop quiz, hot shot: what's the difference between rsync --exclude=src and rsync --exclude=./src?
Pretty significant, actually: the first one will exclude any directory named src, in the whole tree. The second one will only exclude a top-level directory (relative to the directory where rsync was called) named src.
Right now, CollectPathsToExcludeEvent::addPathsRelativeToProjectRoot() uses the first form. In certain situations -- specifically, GitPod developments of Project Browser, where its top-level src directory is cloned into the project root -- that results in rsync getting called with --exclude=src, which causes the stage to have NO src directories in it at all. 💩
Proposed resolution
Composer Stager fixed this upstream in 2.0.0-alpha2, so let's update to that.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | Screenshot 2023-06-07 at 4.59.26 PM.png | 75.79 KB | omkar.podey |
Issue fork automatic_updates-3365151
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
phenaproximaOh, this will also need an upstream follow-up to ensure that Composer Stager's PHP file syncer also treats its exclusions the same way rsync does. Consistency is crucial here.
Comment #4
phenaproximaComment #5
tedbowComment #6
omkar.podey CreditAttribution: omkar.podey at Acquia commentedComment #7
omkar.podey CreditAttribution: omkar.podey at Acquia commentedi just reverted my change, seems like i'm missing something, wouldn't leaving the slashes in the prefix make relative paths appear to be absolute?
Comment #8
omkar.podey CreditAttribution: omkar.podey at Acquia commentedComment #9
omkar.podey CreditAttribution: omkar.podey at Acquia commentedthe excluded paths lists looks like, some of them now have paths staring from
./but i don't think they are being actually excluded because of the failing tests.Comment #10
omkar.podey CreditAttribution: omkar.podey at Acquia commentedMaybe I shouldn't have added the
./to the file names and only to the directories.Comment #11
omkar.podey CreditAttribution: omkar.podey at Acquia commentedis this going in the right direction ? or am i missing something?
Comment #12
Wim LeersThe issue summary only talks about the
rsyncfile syncer, not thephpone.How does this affect that? AFAICT it shouldn't affect that at all, because we're not collecting a list of patterns to exclude, but a list of paths to exclude. This becomes clearer by reading
\Drupal\package_manager\PathExcluder\GitExcluder::excludeGitDirectories(), which if we were excluding patterns wouldn't have to first find all paths to exclude: it could then just exclude.gitand be done.Isn't this just a bug in Composer Stager? It's either a bug in:
\PhpTuf\ComposerStager\Infrastructure\Service\FileSyncer\RsyncFileSyncer? AFAICT that's just misinterpreting thePathListInterface $exclusionsit receives.\PhpTuf\ComposerStager\Infrastructure\Value\PathList\PathList::assertValidInput()Comment #13
tedbowAgree with #12 we need an issue in Composer-Stager. Depending on how quickly a fix can be implemented there we should wait for that or not
Comment #14
phenaproximaOpened https://github.com/php-tuf/composer-stager/issues/183 against Composer Stager.
Assuming this gets fixed upstream (and I think it is likely that it will be), we should probably just close out this issue once we have updated to a new release that contains the bugfix. But for now, postponing on that.
Comment #15
tedbowComment #16
phenaproximaComment #17
phenaproximaComment #18
tedbowLooks good
Comment #20
phenaproximaComment #21
Wim Leers2.0.0-alpha2
Comment #22
Wim Leers