search

rubenv / grunt-git

Git commands for grunt.
MIT License
227 stars 83 forks source link

Updated how command read the list of files. #83

Open toddhgardner opened 10 years ago

toddhgardner commented 10 years ago

changed file arguments to loop over task.data.files.src. Updated common test fixture to construct the fake task with this data structure.

dylancwood commented 10 years ago

Thanks! I will compare your solution to the one that I came up with, which involved removing task.files[i].orig.cwd from task.files[i].src. On Oct 24, 2014 7:26 AM, "Todd H. Gardner" notifications@github.com wrote:

changed file arguments to loop over task.data.files.src. Updated common

test fixture to construct the fake task with this data structure.

You can merge this Pull Request by running

git pull https://github.com/TrackJs/grunt-git master

Or view, comment on, or merge it at:

https://github.com/rubenv/grunt-git/pull/83 Commit Summary

  • changed file arguments to loop over task.data.files.src. Updated common test fixture to construct the fake task with this data structure.

File Changes

Patch Links:

— Reply to this email directly or view it on GitHub https://github.com/rubenv/grunt-git/pull/83.

dylancwood commented 10 years ago

Hi @toddhgardner,

Thanks for pointing out task.data.files. Here is a rather lengthly example to illustrate some potenital issues that I see with this approach. The basic idea is that the dist/ directory contains a separate git repository, and we want to commit everything that does not have a .exclude suffix.

Here's the relevant portion of the sample Gruntfile.:

gitcommit: {
            dist: {
                options: {
                    expand: true,
                    cwd: 'dist',
                    message: "foo"
                },
                files: [{
                    expand: true,
                    cwd: 'dist',
                    src: ['**/*', '!*.exclude*']
                }]
            }
        }

Assume a directory structure that looks like this:

project/
    Gruntfile.js
    dist/
        foo.exclude
        foo2.js
        test/
            test.html

When this task is run, the task.data.files and task.files look like this:

// task.data.files: 
[ { expand: true, cwd: 'dist', src: [ '**/*', '!*.exclude' ] } ]

// task.files: 
[ { src: [ 'dist/foo2.js' ],
    orig: { expand: true, cwd: 'dist', src: [Object] },
    dest: 'foo2.js' },
  { src: [ 'dist/test' ],
    orig: { expand: true, cwd: 'dist', src: [Object] },
    dest: 'test' },
  { src: [ 'dist/test/test.html' ],
    orig: { expand: true, cwd: 'dist', src: [Object] },
    dest: 'test/test.html' } ]

The issue with looping over task.data.files.src, and adding each string to the args array is that we would add both **/* and !*.exclude' (e.g.git commit -m 'foo' / !.exclude`). This will cause issues in almost every environment. In some cases, the OS will take the globbing patterns separately, and expand them into actual file lists that get passed to git commit. However, even then, they will not actually exclude the foo.exclude file.

So... I think that is is necessary to iterate over task.files. In order to make that work, we will need to either use task.files[i].dest, which seems to omit the cwd from the path, or manually strip the portion of the path that is the cwd. This is a solution that I'm working on now, but am getting hung up on a robust way to test it.

Please let me know if you have any suggestions, or if I am misinterpreting the problem that you're trying to solve.

toddhgardner commented 10 years ago

Thanks for the detailed response @dylancwood!

The problem that we are running into is this: assume this structure:

project/
    Gruntfile.js
    dist/
        package.json
        foo.js
        bar.js

We make changes to foo.js and pakcage.json, but nothing else, and need to add them back to the repo. When we reference the changed files explicitly like this:

options: { cwd: 'dist' },
files: { src: ['foo.js', 'package.json']

The only file that is ever detected is the first listed in the array. I am not familiar enough with the internals of grunt to know why this is, but checking the data addressed our problem.

I understand you have other use cases, no worries at all :) Thanks for writing this, it helped us a lot.