Robert-W / grunt-ftp-push

Deploy files to an FTP server as part of your Grunt workflow.
MIT License
39 stars 14 forks source link

New directory creation #9

Closed pyrsmk closed 10 years ago

pyrsmk commented 10 years ago

According to #6, a new directory is not created when pushing files at the root of the dest. It could be already fixed by #6.

There's also another case that when we're pushing files and directories into a distant directory (let's say test), ftp_push tries to create the missing directories into test but get a 550 error.

Robert-W commented 10 years ago

@pyrsmk Thanks for putting this in here, I will try to get this addressed as soon as possible.

Robert-W commented 10 years ago

@pyrsmk Any chance you could provide some sample config items. I have a fix for creating the directories specified in dest that needs a little polishing to handle creating multiple directories if needed. At the moment if your dest is 'test/', it will create the test directory if its not there, but will fail if you specify 'test/html/' and test is not there, working on getting that fixed soon and then will push a new version with this fixed.

Robert-W commented 10 years ago

This is now working with multiple directories so 'test/html/someDir/' will now create test, test/html, and test/html/someDir. Will do some final testing before pushing up a new build.

Robert-W commented 10 years ago

@pyrsmk New version pushed now which should fix this and #7 , if the problem still exists let me know the configuration and I will reopen this ticket.

pyrsmk commented 10 years ago

Thanks a lot! I will test this ASAP.

pyrsmk commented 10 years ago

Now, with the last update, I have a big 501 error : invalid number of arguments width the same code I have. Here's the related code :

        ftp_push: {
            root: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{
                    expand: true,
                    cwd: '.',
                    src: [
                        '.htaccess',
                        'define.php',
                        'index.php',
                        'styles.min.css',
                        //'templates/**/*'
                    ]
                }]
            },
            app: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/app/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{
                    expand: true,
                    cwd: 'app',
                    src: [
                        '**/*',
                        '!images/**/*',
                        '!pictos/**/*',
                        '!scss/**/*',
                        '!ftp.json'
                    ]
                }]
            },
            fonts: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/fonts/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{expand:true,cwd:'fonts',src:['*']}]
            },
            framework: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/framework/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{expand:true,cwd:'framework',src:['**/*']}]
            },
            images: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/images/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{expand:true,cwd:'images',src:['**/*']}]
            },
            js: {
                options: {
                    host: '<%= ftp.host %>',
                    dest: '<%= ftp.dest %>/js/',
                    username: '<%= ftp.username %>',
                    password: '<%= ftp.password %>'
                },
                files: [{
                    expand: true,
                    cwd: 'js',
                    src: [
                        '**/*',
                        '!node_modules/**/*',
                        '!ender*',
                        'ender.min.js',
                        //'!script.js'
                    ]
                }]
            }
        }
Robert-W commented 10 years ago

@pyrsmk When does it throw the error. Is it successfully authenticating and then throwing the error or is this error on authentication?

I don't see anything in your configuration that looks incorrect.

Robert-W commented 10 years ago

Also can you post the console output when this happens.

pyrsmk commented 10 years ago

I cannot post the console output because MinGW doesn't allow me to copy it. But when I do a grunt ftp_push:root, it successfully authenticates and throws the error directly after it.

Robert-W commented 10 years ago

Ok, I will try with the config your using for root and see if I can recreate this today.

Robert-W commented 10 years ago

Just out of curiosity, does it fail when you run other tasks as well or is it just specifically that one?

pyrsmk commented 10 years ago

It fails with all ftp_push tasks.

Robert-W commented 10 years ago

hmm, so I tried a structure very similar.

sample: {
  options: {
    authKey: "test",
    host: "staging.blueraster.com",
    dest: "test/html/",
    port: 21
  },
  files: [
    {
      expand: true,
      cwd: '.',
      src: [
        '.gitignore',
        'package.json',
        'README.md'
      ]
    }
   ]
}

and it does not seem to throw any errors. I tried googling the 501 invalid number of arguments and people seem to be suggesting access issues, which is odd since your getting authenticated but then it is failing. I would recommend in the mean time downgrading to a stable version that is working for you. I am going to keep testing some various things and try to recreate this error. I apologize for any inconvenience this is causing you and really appreciate your help in resolving these issues.

Robert-W commented 10 years ago

Does it output anything to the console after authentication or is the last thing you see

{username} successfully authenticated!

The workflow is it authenticates, creates destination directories if they don't exist, then uploads the files so I am trying to narrow down which part of the workflow is causing the issue and then I target my testing to that section

pyrsmk commented 10 years ago

It does not print anything more. Do you think it comes from jsFTP?

Robert-W commented 10 years ago

I'm not sure, I have just upgraded my local copy to 1.3.1 and everything is working on my side. I can push a new release here shortly to see if that fixes it but my hunch is it won't. What value are you using for destination. If it successfully authenticates, the next thing is it creates destination directories if they don't already exist so It sounds like that may be where the error is coming from.

Robert-W commented 10 years ago

after that it should be just pushing files as usual and that code hasn't really changed much there so I don't think the error is there either.

pyrsmk commented 10 years ago

So it could be my dest parameter... The actual destination I'm using is /hybride_sce2 (and the directory already exists).

Robert-W commented 10 years ago

Ok @pyrsmk, I think I found it. I got a 501 error using a similar destination. Looks like it is the first slash before the directory that was tripping it up. I am now getting it to wrk without the 501 error. I will be pushing a fix later tonight.

Robert-W commented 10 years ago

@pyrsmk Can you please test the new version and let me know if that fixes it. I was getting different error messages then you but I was getting the same 501 error code and that issue is fixed.

pyrsmk commented 10 years ago

Now it works, but with another bug :

Running "ftp_push:app" (ftp_push) task
>> chatnoir successfully authenticated
>> /hybride_sce2/app/app/controllers.php failed to transfer because Error: 550 /hybride_sce2/app/app/controllers.php: No such file or directory
Fatal error: read ECONNRESET

The issue comes from here :

app: {
    options: {
        host: '<%= ftp.host %>',
        dest: '<%= ftp.dest %>/app/',
        username: '<%= ftp.username %>',
        password: '<%= ftp.password %>'
    },
    files: [{
        expand: true,
        cwd: 'app',
        src: [
            '**/*',
            '!images/**/*',
            '!pictos/**/*',
            '!scss/**/*',
            '!ftp.json'
        ]
    }]
}

The dest is /hybride_sce2/app/ but /hybride_sce2/app/app/ is picked. Maybe it's the cwd option that is messing up the computed path?

Robert-W commented 10 years ago

Ah yes you would be correct. I need to write some unit tests so I remember to test all these different cases. I will have a quick fix for that shortly and hopefully, the final fix. I appreciate you bearing with me through this.

pyrsmk commented 10 years ago

No problem ;)

If I'm right, the dest should always be used for the remote side, and the cwd for the local side?

Robert-W commented 10 years ago

Yea, I pulled in a change from another recommendation and forgot to test the cwd with lots of various paths, so thats my oversight, Ill post up here when the new change is good

Robert-W commented 10 years ago

Ok @pyrsmk, I just published a new version, version 0.2.3, I reverted to my original code and it worked but then realized that it only worked in the case of the cwd being one level down and then it broke. It could handle a cwd of '.' or 'test' but 'test/expected' would fail. So it looks like this was an uncaught bug in an earlier version so I appreciate the catch. All these cases should be fixed now .

pyrsmk commented 10 years ago

Neat! It works well. I'll let you know if I encounter other bugs. Thanks a lot for your reactivity, a watcher coupled with a powerful/simple FTP uploader is really needed for me (as I need to test my websites online with the remote database itself). That will save me a lot of time ;)

Robert-W commented 10 years ago

Awesome, great to hear. Sorry if this caused any inconveniences but this has been a big help in getting this plugin polished up and working nicely. I will be writing Unit Tests for future releases so hopefully nothing will be breaking like this again moving forward.

pyrsmk commented 10 years ago

Hum, I think it comes from the same piece of code. I've got another bug with files at the root FTP directory (from my ftp_push:root task). The first dot in the file is deleted.

This files :

.htaccess
define.php
index.php
styles.min.css

Are uploaded like this :

htaccess
definephp
indexphp
stylesmin.css
Robert-W commented 10 years ago

ahh yes I see, all the more reason I need to implement these unit tests asap. When I made the updates I tried to test all the cases I could remember, I tested ./ but not . which is giving me the same issue. I will push an update in the next few minutes. Im going to start adding all these test cases to my unit test ticket.

pyrsmk commented 10 years ago

Nice to hear ;)

Robert-W commented 10 years ago

Apologies about all this. So I have a fix, I have tested with a cwd of '.', './', 'test', and 'test/someDir', I also made sure to test that if the dest option is '/', 'test', or 'test/someDir' files will get pushed to the expected locations and any directories not created will get created. This is pushed as version 0.2.4. I hope this resolves everything and I did not miss a test case. If all is working well, I will update the readme file to include some more examples of things that have been tested.

pyrsmk commented 10 years ago

Neat! All is working as expected. Again, thanks a lot for your reactivity. This library is going to be badass :p

Robert-W commented 10 years ago

Awesome, I sure hope so. Thanks again for the support.