Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does not work with GUI's (e.g. smartGit) #8

Closed
codylindley opened this issue Oct 8, 2013 · 8 comments
Closed

Does not work with GUI's (e.g. smartGit) #8

codylindley opened this issue Oct 8, 2013 · 8 comments
Labels

Comments

@codylindley
Copy link

Seems that it should be stressed that this might not work for developer's that make use of a GIT GUI. It's usage assumes the command line. Or am I wrong? Issue I see is this forces all developers working on the code to work only with GIT commands, on the command line. I bounce back and forth, because conflicts and logs are faster for me in smartGit. But this breaks commits in smartGit.

On the other hand, could this not be updated to deal with git GUI's?

http://stackoverflow.com/questions/17538460/using-git-pre-commit-hooks-in-context-of-github-client

@codylindley
Copy link
Author

Seems the full path to node is needed. And, even maybe to grunt.

@romaricpascal
Copy link
Contributor

Interesting, didn't know about the different behaviours with GUIs. I'd like the plugin to work with GUIs too, so I'll look into a solution for this.

I made a quick test in the issue-8 branch to put the full path to Grunt in the hook (obtained from the argv array). If you want to give it a try, you can replace the grunt-githooks dependency in your package.json with:

"grunt-githooks": "git://github.com/rhumaric/grunt-githooks.git#issue-8"

In the meanwhile, I've updated the README to specify the use cases the plugin is still struggling with :)

@codylindley
Copy link
Author

Well, I think that is one of the issues. The next issue is my git GUI is choking on the node environment, guess because its not the correct path to node either.

@romaricpascal
Copy link
Contributor

Quick try to put the node path in the hook too, it's on the same issue-8 branch.

You might need to clean npm cache for grunt-githooks (npm cache clean grunt-githooks) before reinstalling the plugin. Or use the commit SHA directly:

"grunt-githooks": "git://github.com/rhumaric/grunt-githooks.git#039d152e3146cd844f6ce93389a79760c42e124d",

If you want to make sure you have the right version, the files in the templates directory of the plugin should have a {{nodePath}} placeholder.

Does that solve the problem with the NodeJS path ?

@codylindley
Copy link
Author

might be fast if you quickly tested it with http://www.syntevo.com/smartgithg/

@romaricpascal
Copy link
Contributor

Indeed :) Was just trying some quick fixes before stopping work on the project for the evening.

I'll have a deeper look at it, hopefully later this week, and test it properly with a couple of different GUIs to see if they all behave the same regarding the paths to Node and Grunt.

@romaricpascal
Copy link
Contributor

I took a bit more time to investigate the issue, and test the behaviour of the hooks with various GUI. The hooks worked fine with SmartGit 4.6.4 on Windows and 4.6.2 on Ubuntu. Is this the version you're using?

If it is a Mac OS X specific issue, I will have trouble investigating it as I don't have a Mac. Could you check it is indeed a matter of providing the full path to NodeJS and Grunt? You can manually edit the hook file in the .git\hooks folder with the full paths to do a quick test.

If this is the case, I think I'm going to change the way to address the issue. I don't think it's the plugin's responsibility to insert full paths in the generated hooks to overcome shortcomings of a 3rd party GUI on a specific platform. The plugin should however provide the necessary options to make things works with this GUI. So I'm thinking of a new option to let people define the path to Grunt. This will let the devs decide if they need the full path or not, according to the tools they use around Git. It will also let them decide of the way to get this path (hardcoded in the Gruntfile, detected from process.argv, other...). And as a bonus, they'll be able to hook other commands if they want. What do you think ?

I think it'd also be useful to raise a bug to SmartGit so that their GUI make hooks run in a similar fashion as when run with the Git CLI.

@weotch
Copy link
Contributor

weotch commented Feb 5, 2014

FWIW, I struggled with this for a bit. Here's the config that ended up working for me, based on stuff i found in the README:

githooks: {

    // Use shell instead of node because GUIs like Tower don't see node
    options: { 
        hashbang: '#!/bin/sh',
        template: 'node_modules/grunt-githooks/templates/shell.hb',
        startMarker: '## GRUNT-GITHOOKS START',
        endMarker: '## GRUNT-GITHOOKS END',
        command: 'PATH='+process.env.PATH+' grunt',
        args: '--no-color'
    },

    // Run commands
    install: {
        'post-merge': 'shell:install shell:migrate'
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants