Same named directories in file path cause issue on re-install

[CNanninga]

I have the following situation: I've already run my install the first time, meaning all files from Magento modules have been copied into the codebase (copy, not symlink) and the composer.lock file is already present. On a new clone of this codebase where the vendor directory doesn't exist, I run "composer install" to pull down the components into vendor. This results in the modman-based copy operation being performed again, but a problem happens only in the specific case where the same directory name occurs twice in a row in one of the file paths. Example:

One of my dependency repos has the following line in modman:
app/code/community/CLS/Widgets app/code/community/CLS/Widgets

One of the files there is this one, which has already been copied into the codebase on the first install:
htdocs/app/code/community/CLS/Widgets/Model/Widget/Widget/Instance.php

Notice "Widget/Widget" in the file path. This copy operation worked fine on the first install, but when running "composer install" again with the files already in place, I end up with the following file in my codebase:
htdocs/app/code/community/CLS/Widgets/Model/Widget/Instance.php

i.e., the extra "Widget" in the file path was somehow removed in the copy destination, and the file was copied into the wrong place.

[Flyingmana]

could you please point to a public available module and a workflow how to reproduce the problem?

[CNanninga]

Sure. First, take a look at this repo:

https://bitbucket.org/cnanninga/magento-example

You'll notice that the only model file is located at app/code/local/Nanninga/Test/Model/Widget/Widget/Instance.php

You can test the bug by cloning this repo:
https://bitbucket.org/cnanninga/composer-test

. . . which contains the module files already copied into a Magento codebase. Once you've cloned this, run "composer install" from the root. A git status should show immediately that there is a new file at htdocs/app/code/local/Nanninga/Test/Model/Widget/Instance.php, which is a result of the Magento Composer Installer copying Widget/Widget/Instance.php here instead of the right location.

[davidalger]

Here the test case I'm using to debug this. The call to git status should result in nothing, but shows a new file, which is not present in the test magento-module repository. The fix in commit c812c5c actually introduces a different sort of replication bug when the modman paths differ, so I've got to chase that one down now… unless someone else has a solution. ;)

dalger:04:45 PM:/server/proj$ git clone https://github.com/davidalger/composer-basename-bug-mage.git
Cloning into 'composer-basename-bug-mage'...
remote: Counting objects: 16161, done.
remote: Compressing objects: 100% (7519/7519), done.
remote: Total 16161 (delta 6641), reused 16161 (delta 6641)
Receiving objects: 100% (16161/16161), 19.87 MiB | 3.63 MiB/s, done.
Resolving deltas: 100% (6641/6641), done.
Checking out files: 100% (12063/12063), done.
dalger:04:49 PM:/server/proj$ cd composer-basename-bug-mage/
dalger:04:50 PM:/server/proj/composer-basename-bug-mage$ composer install
Loading composer repositories with package information
Installing dependencies (including require-dev) from lock file
  - Installing magento-hackathon/magento-composer-installer (dev-master 2.0.0-beta1)
    Cloning 2.0.0-beta1

  - Installing davidalger/composer-basename-bug-case (dev-master fc5b0d6)
    Cloning fc5b0d6d30523523885632101bb055de670c2ec7

magento-hackathon/magento-composer-installer suggests installing magento-hackathon/composer-command-integrator (*)
Generating autoload files
dalger:04:50 PM:/server/proj/composer-basename-bug-mage$ git status
# On branch master
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#   htdocs/app/Foobar/Widget.php
nothing added to commit but untracked files present (use "git add" to track)
dalger:04:50 PM:/server/proj/composer-basename-bug-mage$ ls -1R htdocs/app/Foobar/
Foobar
Widget.php

htdocs/app/Foobar//Foobar:
Widget.php
dalger:04:50 PM:/server/proj/composer-basename-bug-mage$ 

[Flyingmana]

will not get fixed, as it is in original modman only possible with --force and would make problems there, too. (as it simple overwrites)
So $project should fix their modman file instead.

https://twitter.com/kalenjordan/status/449722175851753472

[davidalger]

@Flyingmana I completely disagree with what you're saying, and for the record, that is completely unrelated to the issue at hand. This isn't an issue with linking failing because something is already there, the bug is that the copy (not link, it doesn't affect the link method) is putting the files in the wrong place.

If you look at commit 7a9ff2d on my fork, you'll see that I have implemented a unit test which demonstrates the bug. Take a look if you will, but this is absolutely not due to having a broken modman file.

Are you trying to say that you not accept a pull-request which resolves this bug?

[Flyingmana]

linux cp behavior for cp -R some/dir/path some/other/path: results in some/other/path/path

what I say is, that I dont accept the initial reported issue as a bug.
This says nothing about my acceptance of pull requests. But I probably will compare the changes with the original modman behavior.

[colinmollenhour]

This is why I don't like the --copy option, there are too many conflicts since you can't easily tell the difference between a file that is safe to delete and one that is not (whereas symlinks are pretty much always safe to delete). So for the people that want to use --copy, I think the only way is to essentially assume --force. That is if some/other/path already exists before the copy, it should be removed. Dangerous, yes, which brings me back to just sticking with symlinks.

[kalenjordan]

If I'm following this correctly, the root cause of this issue is:

linux cp behavior for cp -R some/dir/path some/other/path: results in some/other/path/path

So it's not technically speaking a bug in modman or the composer module, it's a limitation of the implementation of cp. And @davidalger implemented a workaround for this in the composer copy deploy strategy in c812c5c (with bonus points for including a test case to demonstrate the fix)

But @Flyingmana you don't want to merge that fix?

I haven't reviewed the code in detail or tested it, but if the patch resolves the issue, I can't see why you wouldn't want to merge it?

[Flyingmana]

please comment the pull request related comments on pull request :)

[kalenjordan]

Oops sorry I was linked directly here from twitter - I can see on the PR that tests are failing and that seems to be the only thing holding it up!

Thanks Daniel!

[davidalger]

@kalenjordan Yeah… still working on figuring out an actual solution. Forgot to run unit tests on my first pass. :/ I've got 2 failing tests still.

@Flyingmana Ok. I think you're misreading the issue then. :) The root cause is not due to behavior of linux cp. I understand how that works, this is different. And I'll also note that implementing a test to reproduce the bug requires the use of the force flag too.

I've created a 2nd branch on my fork "issue-76-tests" which has the unit test isolated from my modifications in order to demonstrate the bug, then I added calls to tree so it will print the output for debugging purposes.

Here is the output from the test:

dalger:04:35 PM:/server/proj/magento-composer-installer$ phpunit --filter testCopyDirToDirOfSameName
PHPUnit 4.1-dev by Sebastian Bergmann.

Configuration read from /Volumes/Server/proj/magento-composer-installer/phpunit.xml.dist

F

 -- 1st pass tree
/usr/local/zend/tmp/testCopyDirToDirOfSameName/magento_dir/dest/root
└── subdir
    └── subdir
        └── test.xml

2 directories, 1 file


 -- 2nd pass tree
/usr/local/zend/tmp/testCopyDirToDirOfSameName/magento_dir/dest/root
└── subdir
    ├── subdir
    │   └── test.xml
    └── test.xml

2 directories, 2 files


Time: 135 ms, Memory: 5.75Mb

There was 1 failure:

1) MagentoHackathon\Composer\Magento\Deploystrategy\CopyTest::testCopyDirToDirOfSameName
Failed asserting that file "/usr/local/zend/tmp/testCopyDirToDirOfSameName/magento_dir/dest/root/subdir/test.xml" does not exist.

/Volumes/Server/proj/magento-composer-installer/tests/MagentoHackathon/Composer/Magento/Deploystrategy/CopyTest.php:54
phar:///usr/local/Cellar/phpunit/3.7.28/libexec/phpunit-3.7.28.phar/phpunit/TextUI/Command.php:176
phar:///usr/local/Cellar/phpunit/3.7.28/libexec/phpunit-3.7.28.phar/phpunit/TextUI/Command.php:129

FAILURES!                            
Tests: 1, Assertions: 2, Failures: 1.
dalger:04:35 PM:/server/proj/magento-composer-installer$ 

Note that in the 2nd tree which is printed, the file test.xml shows up twice. This should not be the case. In summary, the nodes in the path with the same name are causing trouble walking the path because basename is being used to identify whether or not it must descend or iterate contents. On the first pass everything is fine because nothing exists and it simply does a recursive copy vs descending into the directory tree and copying everything individually.