Deploying project copies /node_modules into output folder

[gregmsanderson]

Deploying project copies /node_modules into output folder

[t-geindre]

The deployer should also check if the vendor-dir is defined in composer.json and uses its value if so.

[gregmsanderson]

Ah ... I think as a result of this I have spotted a bug. So ok, eventually it did copy the whole node_modules folder to .bref/output. Which I'm not entirely sure it should do, by default, but with a build hook it would need to, given how it copies to .bref/output and then runs the build hook on that copy of it.

So that's currently not a problem since it means only the initial deploy is slow, and I assume the node_modules is excluded once serverless takes over the .zip part anyway, so it doesn't get sent up to Lambda.

Anyway, so I mentioned the possible bug and it is this: I kept getting an error in the console when it ran my build hook command (to minify CSS, optimise images etc). Couldn't figure it out, but it was complaining about the imagepng command so I looked in the node_modules folder generated within .bref/output, and noticed that in that (grey arrow), the /node_modules/optipng-bin/vendor folder was not present. Whereas in the original version in the main project folder (as shown in the attached image, black arrow), the /node_modules/optipng-bin/vendor folder is present. So that would explain why a build hook command works outside of bref, but not when run as part of a deploy: it is using a different version of node_modules.

So the question then was why the vendor sub-folder was excluded from the mirror. I suspect it could be because of these lines which I see were added to stop the PHP /vendor folder from copying into the output folder (to let bref handle that). However it seems like it could be stopping any folder called vendor copying across. And as shown in this example, there are other vendor folders that do need to be copied and so should not be excluded. So maybe that part needs adapting to only exclude folders called 'vendor' at the root level (if that is what is causing it, as seems likely)?

https://github.com/mnapoli/bref/blob/master/src/Console/Deployer.php#L243

[mnapoli]

I took a look at the copyProjectToOutputDirectory function in Deployer.php and it seems to not respect the include/exclude part of serverless.yml, as it copies the whole project folder (well, apart from files it knows not to, like vendor). I assume that include/exclude only then gets processed when deploying the zip?

Yes. The include/exclude setting is for the serverless deploy process which happens later on.

However it seems like it could be stopping any folder called vendor copying across.

Oh right that is very probably a bug then!

[mnapoli]

The line:

https://github.com/mnapoli/bref/blob/3b2c8e268b22a9a4e677a00d2bbff42a51b1775e/src/Console/Deployer.php#L243

That is weird the documentation says:

Directories passed as argument must be relative to the ones defined with the in() method.

https://api.symfony.com/4.1/Symfony/Component/Finder/Finder.html#method_exclude

[gregmsanderson]

Yep. I agree - with the Symfony finder, it does sound like it should only be excluding the 'root' vendor folder, and not any folder called vendor, however like I say, it's odd the one folder missing from my deploy was one called vendor. So it may not be because of that but seems likely. Maybe we need to set up a test script (like I did before, testing the Symfony progress), copying a folder that contains /vendor at the root and a sub-folder containing /vendor, run the Symfony finder, and see if it does work as expected

[gregmsanderson]

As I thought, the Finder is not working as we all expected. This issue reports the same problem: symfony/symfony#28158
... ie that they excluded a folder called 'data' and expected only the 'root' one to be excluded. However they found that any folder called 'data' was excluded. Exactly what is happening with your exclude of 'vendor'.

So until/if they change that behavior, a possible solution (a little inefficient, but given the amount of other folders being copied like node_modules being copied around, I guess one more doesn't hurt) would be to let it be copied across (in the mirror) and then delete it after.

So remove the exclude of 'vendor' lines:
->exclude('vendor')
from 243 and 250 of Deployer.php

... then, since the /vendor folder is now copied - which we don't want - after the
$directoryMirror->mirror($source, $target);
line, simply delete it. This time using the bref-managed PHP, so it will delete just the 'root' /vendor and not any sub ones. So using some kind of function to delete a folder and its contents like this one found from a quick Google:
$this->deleteDirectory('.bref/output/vendor');

function removeDirectory($path) {
 	$files = glob($path . '/*');
	foreach ($files as $file) {
		is_dir($file) ? removeDirectory($file) : unlink($file);
	}
	rmdir($path);
 	return;
}

?

That's what I'm going to do anyway.

Bonus: as @t-geindre suggests, do:
$this->deleteDirectory('.bref/output/'.$vendor-dir);
(or whatever - not assuming it is called /vendor. In my case it is so I'm fine hard-coding it, but that would be handy for anyone who changes it to something else)

[gregmsanderson]

Turns out that function removeDirectory throws a bunch of PHP warnings on Ubuntu about deleting non-empty directories, I guess because of the glob approach including '.' files. So I swapped it for this which seems to work, deleting the /vendor folder so the function ends as if it had not been copied in to the folder in the first place (so if the Finder worked as expected)

$directoryMirror->mirror($source, $target);
        
        $this->removeDirectory('.bref/output/vendor');
    }
    
    private function removeDirectory($dir) {
        if (is_dir($dir)) { 
            $objects = scandir($dir); 
            foreach ($objects as $object) { 
                if ($object != "." && $object != "..") { 
                    if (is_dir($dir."/".$object))
                        $this->removeDirectory($dir."/".$object);
                    else
                        unlink($dir."/".$object); 
                }
            }
            rmdir($dir);
        } 
    }

[mnapoli]

Closing since this issue is obsolete for v0.3 (deployment happens through AWS SAM).