set_shared_root in cli.sh doesn't work with repo in scp form

[magjoh]

Configuring a global shared hook in scp form such as git@github.com:shared/hooks-maven.git causes set_shared_root in cli.sh to keep DO_SPLIT as "true" and will result in the error below:

* Retrieving shared hooks from: git
! Clone failed, git clone output:
fatal: repository 'git' does not exist

Forcing DO_SPLIT="false" correctly clones the repo when issuing the command

git hooks shared pull

[rycus86]

Pinging @gabyx to see if you recall what is that splitting business for there? Is it for splitting off auth info from private repo URLs or something?

Thanks for the example @magjoh , we should be able to put it in a test and get it fixed.

[gabyx]

Thanks for reporting!, I will first fix this in the successor: https://github.com/gabyx/githooks and will report back what the problem is. I am almost done, that I can release version 1.0.0. launching of this baby.

[rycus86]

Thanks! If you found out what's wrong, it might be good to backport here too, assuming it's not something too tricky. 🙂

[gabyx]

See: https://github.com/gabyx/githooks/pull/7

SCP Regex: should probably be:

// reShortSCPSyntax is the regex for a short scp syntax.
// The problem arises on Windows with drive letters, since `C:/a/b`
// can be technically be a short SCP syntax, we require at
// least 2 letters for the host name.
var reShortSCPSyntax = regexp.MustCompile(`(?m)^(?P<user>.+@)?(?P<host>.+[^:]):(?P<path>[^:].*)`)

I will not backport, because I will not maintain this anymore since the GO successor is easier/better/faster to maintain. I hope you understand. I am almost ready for first version. At the moment, updating works flawlessly. (+ we have propers windows tests, which just popped up an ugly error: c:/a/b/c can be a short SCP syntax as well 🙈 )

[magjoh]

I forgot to mention that a clone URL of ssh://git@github.com/shared/hooks-maven.git didn't work either but maybe that's part of the fix?

If this is not going to be backported, should I switch to using the gabyx version (I can live with my workaround, but for future updates is it better to switch)?

[gabyx]

I would switch once the first version is out! For both our precious time and sanity, is better to work with the Go port, because its more reliable and testable and has new and improved features. (parallel + better ignoring hooks/files) You can already star the project. To give it a bit of support.

[rycus86]

Thanks for the regex @gabyx ! I'll try to add it in this repo this weekend.

[gabyx]

There is more wrong stuff happening, needs more tests. I'll report back

[gabyx]

@rycus86 Do you remember why we didnt split the @ on bare repositories? I dont see any reason why not support that (in the new version, if we clone it anyways, we can split off the last @branch stuff)

https://github.com/rycus86/githooks/blob/6283fabf39ad17ea5e9798f02d3642abfa01b4d5/base-template.sh#L543

[rycus86]

Not too sure to be honest, but yeah maybe you wanted to introduce targeting non-main branches there perhaps?

[gabyx]

@rycus86 : Instead of investing time and your nervs getting this right, I'd would appreciate a read through of the README :-) see: https://github.com/gabyx/githooks/pull/8

tl;dr: -> Before we strip any stupid last '@branch' we need to be sure from what we strip this, and that can only be done by distinguishing between normal URLs or SCP, so for a fast fix: ->

• check for protocol://... -> Normal url -> strip from path as in user@host.com/path@branch. (So you need a URL parsing in bash.... :see_no_evil: )
• otherwise check the SCP regex -> if match strip it from 'address' in user@host:address -> return
• otherwise strip last '@branch'

[gabyx]

Fixed in https://github.com/gabyx/githooks/releases/tag/v1.0.12

[rycus86]

Thanks again for reporting @magjoh - it should be fixed now, just run an update on Githooks. Let me know how it works out!