rm -r if directory exists?!

[zygis]

If linked directory already exists with some files inside, best option delete all?! It's a terrible idea. Never happened before like this with capistrano. Why not copy folder content to ../shared/.. and then do remaining stuff?

[timkelty]

I agree, seems a bit extreme.
Here's where it comes from in cap: https://github.com/capistrano/capistrano/blob/master/lib/capistrano/tasks/deploy.rake#L108

I'm open to a PR to change this, or I can try and look at it soon.

[tlartaud]

I agree with that using rm -R is a bit destructive, but in fact, in some cases, we'll need it.

Let's assume that a user already prepared his shared folder before publishing the first time. the target folder in shared folder will already exist, so the directory will not be copied to the shared folder because something there already exists, so the last option i see is to move the folder in the tmp folder, and to backup/move them somewhere instead of deleting.

Personally, i don't see any problem about rm -R except if the user performed a misconfiguration, but i don't see exaclty how it could remove something since that everything that can be removed there comes from your git repository, so you wont lose any sensitive data in any case, only some datas that you have in your repository.

So, I agree with the idea that rm -R is too much destructive/dangerous, but i don't agree with your actual solution about copying folder to shared folder ( plus : this is only usefull on the first deployment .. On the second publish, the dir will already exists in shared folder, so, you will need to move or delete it. No others choices. )

[tlartaud]

The following structures represent how i use, and how i think we should use symlinks. Let's assume we want to have an uploads folder in our app, but we want it to be stored in the shared folder on the remote.

Local structure

- app
-- uploads
---- file1
---- file2
---- etc

Git structure (files inside uploads are gitignored)

- app
-- uploads
---- .gitkeep

Remote structure

- app
-- uploads -> symlink to shared/app/uploads folder

When shipit-shared perform the rm -r, it's on the git structure. So i don't see why removing an uploads dir that contains a .gitkeep file could cause a problem.

Are you maybe using symlinks in a different way than me ?

[tlartaud]

Plus : alternatively, you can actually add app/uploads to the ignored files array in your shipit config, so no directory will exist in that place after deploying, so your symlink can be created without deleting any file or directory.

[zygis]

How hard it can be?:

mkdir shared/uploads
if (folder current/uploads exists) {
  cp -R current/uploads/* shared/uploads
 // or even:
 mv current/uploads shared/uploads
}
rm -fr curent/uploads
// rest stuff

Most common problem with rm -fr and first deployment:
node_modules shared folder, "shipit-shared" module runs after "shipit-npm", so you have to deploy repo twice... Not very good experience...

Next problem: README.md I can't see any of these word "remove", "rm -fr", "lost", "backup first". So if I have repository and after few months decide to move one folder with lot of files to "shared"... I WILL LOST ALL MY DATA! So this is a problem!

Even stop deployment/trigger error is better option than rm -fr... How I can trust if you "don't see any problem" with deleting my files? Even with automated backups file uploaded 2 min ago will be lost permanently! I glad I found this before something terrible happened.

[timkelty]

Not hard, I just haven't had time, sorry. I'm open to PR, otherwise I'll try and do it this weekend.

[tlartaud]

@zygis You also use rm -fr in your code. I don't see how your solution answer your query about rm -fwhich is too destructive.

Finally, your question is just related about the following feature request, not to delete rm -fr : #10 (add Option to create files if they don't exist).
And the copy should really be an option, and not be executed everytime (not usefull in all cases, or for all users i guess)

[tlartaud]

@zygis

How I can trust if you "don't see any problem" with deleting my files?

Don't worry, i'm not shipit-shared author, you don't have to trust me ;) I'm just a shipit-shared user, and i'm just giving there my mind and advices about this subject.

[tlartaud]

@zygis

node_modules shared folder, "shipit-shared" module runs after "shipit-npm", so you have to deploy repo twice... Not very good experience...

Can you explain more about this ? I personally don't need to publish twice. It's interesting me, can you pls explain in which case and why you'll need to publish twice ?

[tlartaud]

@zygis @timkelty I think that something similar to that could solve the problem

mkdir shared/uploads
if (folder current/uploads exists && folder shared/uploads is empty && option to allow files copying for uploads dir set to true) {
  cp -R current/uploads/* shared/uploads
}
rm -fr curent/uploads
// rest stuff

json

"shared": {
      "baseDir": "shared",
      "symlinkPath": "/var/www/vhosts/domain.com",
      "dirs": [
        { "path": "html/app/uploads", "copy": "once" },
        { "path": "html/app/cache", "copy": "never" },
        { "path": "html/app/wp-rocket-config", "copy": "always" }
      ],
      "files": [
        { "path": ".env", "copy": "once" },
        { "path": "html/.htaccess", "copy": "once" },
        { "path": "html/app/advanced-cache.php", "copy": "never" }
      ]
    }

What do you think ?

[zygis]

@tlartaud

i'm not shipit-shared author

Yes sorry, didn't noticed.

Can you explain more about this ? I personally don't need to publish twice. It's interesting me, can you pls explain in which case and why you'll need to publish twice ?

Just "shipit-npm" completes his job first, but after "shipit-shared" and this rm -fr problem node_modules folder is empty. This problem exist for the first deploy only. But if I have to deploy ~50 micro services to new server - it's pain in ass...

Example above is ok, but most important is "smart defaults", or at least clear documentation :)

[tlartaud]

@zygis

Just "shipit-npm" completes his job first, but after "shipit-shared" and this rm -fr problem node_modules folder is empty. This problem exist for the first deploy only.

I can't understand this sry.i don't get why you're talking about an empty node_modules folder :/

[zygis]

@tlartaud
sequance done on server:

// deploing first time
npm install;
mkdir shared/node_modules;
rm -fr node_modules;
ln -s shared/node_modules node_modules
// deployment done

At this state I can't start application. Why? Because node_modules folder is empty...

module.exports = function (shipit) {
    "use strict";
    require("shipit-deploy")(shipit);
    require("shipit-shared")(shipit);
    require("shipit-npm")(shipit);
    shipit.initConfig({
        default: {
            workspace: "/tmp/repo",
            deployTo: "/path/to/repo",
            repositoryUrl: "git@bitbucket.org:org/repo.git",
            ignores: [".git", "gulpfile.js", "shipitfile.js"],
            keepReleases: 5,
            branch: "master",
            rsync: ["--del"],
            shallowClone: true,
            shared: {
                dirs: [
                    "node_modules"
                ]
            },
            npm: {
                remote: true,
                installFlags: ["--production"]
            }
        },
        production: {
            servers: ["someone@somewhere.com"]
        }
    });
    shipit.blTask("restart", function () {
        return shipit.remote("sudo /sbin/stop service_name;sudo /sbin/start service_name");
    });
};

I have to do: shipit production deploy and then shipit production deploy restart

[tlartaud]

@zygis nodes_modules in a shared folder ? Your app looks strange ^^
do you really need to get your nodes_modules online ? Will you really run grunt tasks directly on production server ?

Tip : You should prefer using your apps like that way so you make sure that folders like node_modules are not accessible from the web without any other apache configuration.

- current
-- public
--- index.php
-- node_modules

(and point your web vhosts to current/public)

Plus : Shipit-shared works like capistrano. the spirit is to build all the last release app or website, get everything compiled locally, and push the result on the remote. So normally, you don't even need to get your nodes_modules or grunt tasks online, on production server. In some cases, with some apps, it could be required to send grunt tasks online, but i think your doing something you could do locally more easily..

Then, why do you need to run grunt tasks on remote prior to first shipit-npm execution ?
Cant you add a task binded to the deploy:published event that run your script ? You can create a command using shipit.remote to simply run npm install remotely after symlinks have been created and that should work, no ?

[tlartaud]

@zygis This is some example tasks to show you to bind event or do some actions locally or even remotely, when you need them.
For example, i run composer online, because composer is dependent from php, and has my php versions & modules installed can be different locally and remotely, i need it to be executed remotely, with the proper php installation.
You also have an example for running a sh script remotely, or you could do the same simply inside a grunt.shipit.remote command.

grunt.registerTask('construct:pre_deploy', function () {
        grunt.task.run([
            'prompt:bump'
        ]);
    });

    grunt.shipit.on('deploy', function () {
        grunt.task.run(['construct:pre_deploy']);
    });

    grunt.registerTask('construct:remote', function () {

        // Shipit-shared is binded to `on published` shipit-deploy event.
        // It will normally be executed before this tasks list.

        // Prepare some vars
        var deployPath = grunt.shipit.config.deployTo,
            releasesPath = grunt.shipit.releasesPath,
            releaseDir = grunt.shipit.releaseDirname,
            releasePath = grunt.shipit.releasesPath + "/" + grunt.shipit.releaseDirname,
            scriptPath = releasePath + "/config/mastack/shell/";


        // Run composer installation
        grunt.shipit.remote(
            "cd '" + releasePath + "' && composer install;",
            this.async()
        );

        // Execute custom Post-deploy Script if it exists
        // We test file existence locally, and assume the file exists on the remote
        if (grunt.file.exists('config/mastack/shell/post-deploy.sh')) {
            grunt.shipit.remote(
                'cd ' + scriptPath + ' && sh post-deploy.sh "' + deployPath + '" "' + releasesPath + '" "' + releaseDir + '"'
            );
        }

    });

    grunt.shipit.on('published', function () {
        grunt.task.run(['construct:remote']);
    });

Hope that helps.

[zygis]

@tlartaud

do you really need to get your nodes_modules online

I don't use apache, and all my applications are REST only, not a single file aren't exposed to public...

So normally, you don't even need to get your nodes_modules or grunt tasks online, on production server

You probably don't know how node.js applications work. node_modules folder contains all dependencies that included in a running application at runtime. Application can't work without dependencies. Theoretically you can run npm install on local side, and then upload to remote, but some of dependencies are using C modules and need to be compiled. Developer with windows definitely can't do that, because code will be compiled for wrong operating system. Same thing with go applications, dependencies should be on remote, and code should be build on that server where you want run application.
Grunt/Gulp tasks yes running local side only.

You can create a command using shipit.remote to simply run npm install remotely after symlinks have been created and that should work, no

Yes :) But fixing unexpected behaviors (or creating issues like this one) is my preferred way :) Just right tools should work right.

[timkelty]

Sorry, I've been really busy and haven't had time to chime in, but I will try and get this addressed this week. And as always, PRs are welcome.

[timkelty]

Review all of this now...beginning to see the issue.

@zygis I think the reason I haven't seen this come up before is I've never seen anyone put their node_modules dir as a shared dir. I've always just had it run npm install in current and leave it at that.

Also, defining node_modules as shared seems a little dangerous to me, as it seems like things might break when rolling back, if there was a dependency change between those deploys.

Having said that:

• You're not sourcing node_modules in your repo, correct?
• If you remove node_modules as a shared directory, do you still have the issue with having to deploy twice?
• It appears that you are using the shipit-npm module, yes?

Furthermore, once I roll in this feature, you would be able to ensure that shipit-shared always runs before shipit-npm

[zygis]

@timkelty

You're not sourcing node_modules in your repo, correct?

Yes.

Also, defining node_modules as shared seems a little dangerous to me, as it seems like things might break when rolling back, if there was a dependency change between those deploys.

Removing node_modules folder or changing it's contents usually is not a problem, because all modules required are already running on RAM, and usually doesn't need to hammer disk. Of course if you have dynamically loaded modules probably this could be issue, but my applications requiring all needed modules at startup. And I know what I am doing (or at least I think I know :) )

If you remove node_modules as a shared directory, do you still have the issue with having to deploy twice?

No. This problem exists only when using shared dirs.

I've always just had it run npm install in current and leave it at that.

Of course you can do that, but migrating lot of processes to new server is totally pain (few times migration was in ASAP mode, due some hardware problems). Other developers in a team can simply forget to do one or another action. It's no problem when you working alone and deploying max 1 time to 1 environment. My usage look like: 6 devs, 2 environments (testing, production), ~50 different repositories and average 5 deployments per developer per day. So building good shipit config file is time/bug saver for all my team.

One of the main reason why I put node_modules folder to shared - build time. Some legacy applications have tons of dependencies, these dependencies have tons other dependencies. Some of these dependencies use C modules. Updating dependencies are MUCH faster than installing from scratch. I agree that my situation isn't very common, but I must find proper solution to this.

[timkelty]

Got it. We'll figure something out that works for you.

I realize your solution pretty easily takes care of your use case, but I'm wary of fixing it that way just because that behavior seems just as unexpected to me.

Updating dependencies are MUCH faster than installing from scratch.

Yes, but since shipit-deploy copies the entire previous release first, it shouldn't be installing from scratch.

[timkelty]

@zygis what about something like this in your shipit config?

  shipit.blTask('move-modules', function() {
    return shipit.remote(
      sprintf('mv %(src)s %(dest)s', {
        src: path.join(shipit.currentPath, 'node_modules'),
        dest: path.join(shipit.sharedPath, 'node_modules'),
      })
    );
  });

  shipit.on('npm_installed', function() {
    shipit.start('move-modules');
  });

npm_installed in an event emitted by shipit-npm.

[tlartaud]

@timkelty since shipit-deploy:cleanup (last task) create the symlink from current to the last release folder, you have to avoid using shipit.currentPath in your task, else you will not be working on the expected release (i mean, not the publishing one).

The following should work on the first remote deploy

 shipit.blTask('move-modules', function() {
    return shipit.remote(
      sprintf('mv %(src)s %(dest)s', {
        src: path.join(shipit.releasesPath, shipit.releaseDirname, 'node_modules'),
        dest: path.join(shipit.sharedPath, 'node_modules'),
      })
    );
  });

  shipit.on('npm_installed', function() {
    shipit.start('move-modules');
  });

[timkelty]

That's right, thanks @tlartaud.

[tlartaud]

@timkelty that said, i can't find any documentation on shitpit-deploy related to the npm_installed event, so i don't know if shipit.releasesPath and shipit.releaseDirname are already set on this event. If this is not working, maybe we can use the published event instead of npm_installed

shipit.on('published', function() {
    shipit.start('move-modules');
  });

[timkelty]

Here's what I'm considering:
On deploy..

• shipit-shared checks if the symlink target in current exists
• if it does, it alerts you, and asks if you want to delete it, copy to shared, or abort.

Would this satisfy both of you? @tlartaud @zygis?

[tlartaud]

@timkelty great idea ! sounds good for me, with some little involves maybe ;)

• shipit-shared checks if anything exists on the target place inside path.join(shipit.releasesPath, shipit.releaseDirname)
• if something is found, it alerts you, perfom a ls -al on the console to show the user what files are in this place
• and asks if you want to
  • delete it
    • and then creates the symlink
  • or move it to shared
    • if nothing found on shared
      • move the folder to shared and then creates the symlink
    • if something found on shared
      • abort moving, (move to tmp_deploy/backups instead) and then creates the symlink
      • or delete shared content and move new one in the right place and then creates the symlink

[tlartaud]

@timkelty and instead of a prompt, it would be better i guess to have the configuration stored for each file association in the config array like all the others shipit-shared configs. So, the config can be stored in the repo for team mates.

For example

"shared": {
      "baseDir": "shared",
      "dirs": [
        { "path": "html/app/uploads", "`ifExists": "delete" },
        { "path": "html/app/cache" },
        { "path": "html/app/wp-rocket-config", "ifExists": "forceMove" },
        { "path": "node_modules", "ifExists": "move" }
      ],
      "files": [
        { "path": ".env", "ifExists": "delete" },
        { "path": "html/.htaccess", "ifExists": "forceMove" },
        { "path": "html/app/advanced-cache.php", "ifExists": "move" }
      ]
    }

and you can set ifExists default to move if not set.
ifExists means if something exists where we try to create the symlink file.

Or, if you don't want to break your actual syntax

"shared": {
      "baseDir": "shared",
      "dirs": [
        "html/app/uploads --ifExists:delete",
        "html/app/cache",
        "html/app/wp-rocket-config --ifExists:forceMove",
        "node_modules --ifExists:move"
      ],
      "files": [
        ".env --ifExists:delete",
        "html/.htaccess --ifExists:forceMove",
        "html/app/advanced-cache.php --ifExists:move"
      ]
    }

[tlartaud]

@zygis @timkelty @kosssi

Hi guys, I've made a first approach about what we were talking about there ...

I've added a --mode option to use in each path, so we can use each file differently.

See commit for more details.
Any comment would be really appreciated.

[timkelty]

Thanks @tlartaud - should get time to dig back into this today!

@zygis - would this fulfill your requirements?

[tlartaud]

cool, do not hesitate to tell me if there is something you do not understand.
my code is not optimal, i'm not a javascript Guru, but i think it may work.
If you merge this, i'll update some code later, and add a new --chmod option, to set chmod on files/directories after they have been copied to the shared folder.
thx

[tlartaud]

@timkelty dude .. you are so damn slow ....

[timkelty]

Sorry man, been slammed with other work.
I'll work on this today and get it merged in.

[timkelty]

Hi all,
Again, sorry for the delay, and I appreciate the pull-requests.

I've thought this through and while I see the benefit of doing things like backing up folders, moving, etc., I think it's overcomplicating what the task should be doing, and adding complexity for a potentially narrow use-case.

The core of this issue was the unexpected and undocumented use of rm -rf, which has been addressed.

What I've come up with is published to 4.0: https://github.com/timkelty/shipit-shared/tree/master

Essentially, I've added an override option that defaults to false. This can be set for all items, or per-item. If a file exists in current when the task tries to create the symlink, the task will abort with an error message. If you want it to go ahead and remove those items and avoid an error, use the new override option.

Furthermore, there is now a shipit.config.shared.triggerEvent option if you want to customize the event flow.

@zygis' case of a shared node_modules directory can be solved by running npm install after shared*:

shipit.initConfig({
  shared: {
    dirs: [
      'node_modules'
    ]
  }
});

shipit.blTask('npmInstall', function() {
  return shipit.remote(util.format('cd %s && npm install --production', shipit.releasePath || shipit.currentPath));
});

shipit.on('sharedEnd', function() {
  shipit.start('npmInstall');
});

• Note: This example does not use shipit-npm. The same thing should be possible using the npm_preinstall event, and manually running shared. I'll post an example of that shortly.

[tlartaud]

@timkelty Wow, this is soooo bad. I'm pretty sure you didn't test what i did.
I give up, i totally give up and will make my own script.

Have a good luck with it.
Bye.

[timkelty]

Wow, this is soooo bad

Can you elaborate? That doesn't really help.

I'm pretty sure you didn't test what i did.

I did. I just didn't agree with the approach for solving this particular problem. The issue stated was that @zygis didn't expect his files in current to be removed (though this is exactly what happens in Capistrano). Users now have to opt-in to this, and it has been documented.

I'm not necessarily opposed to adding features to backup and move things around, but it seemed to me like it was going way beyond the scope of this original issue. I haven't heard a justified use-case from you for these features, and in fact, you argued against it:

So, I agree with the idea that rm -R is too much destructive/dangerous, but i don't agree with your actual solution about copying folder to shared folder ( plus : this is only usefull on the first deployment .. On the second publish, the dir will already exists in shared folder, so, you will need to move or delete it.

The fact remains that @zygis' use of shipit-npm on updated, combined with defining node_modules as a shared directory is what caused his issue. To me, that's a pretty specific scenario to justify a new set of code and features. The original issue has been addressed (opt-in to rm -rf, and documenting it), and a solution to the node_modules issue has been provided. If we want to integrate that into shipit-shared, I'm open to doing that in a separate issue.

I give up, i totally give up and will make my own script.

That's your prerogative. Take your PR and use it, or write something yourself.

[timkelty]

With the latest update to shipit-npm, controlling the order of tasks is now easier:

shipit.initConfig({
  shared: {
    dirs: [
      'node_modules'
    ]
  },
  npm: {
    remote: true;
    triggerEvent: 'sharedEnd'
  }
});

[timkelty]

Closing this issue. Happy to re-open if @tlartaud or @zygis have any constructive input.

Discuss issues related to moving files from current to shared here: #23