rycus86 / githooks

Githooks: per-repo and global Git hooks with version control
MIT License
380 stars 19 forks source link

Go Rewrite #129

Closed gabyx closed 3 years ago

gabyx commented 3 years ago

Progress

A draft PR to have you guys in the boat.

The progress of porting this application could be like this:

gabyx commented 3 years ago

Makeing good progress. Keeping the implementation as close as possible because I think its pretty good as it is. Also to make the test work at the end...

Go is a charm sometimes a bit to easy, but good to keep it that way (miss generics...) Its so much better to go with a proper language than Bash...

Some insights from the runner: image

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 911


Changes Missing Coverage Covered Lines Changed/Added Lines %
base-template.sh 3 4 75.0%
<!-- Total: 5 6 83.33% -->
Totals Coverage Status
Change from base Build 907: 0.0%
Covered Lines: 2371
Relevant Lines: 2955

πŸ’› - Coveralls
gabyx commented 3 years ago

image

gabyx commented 3 years ago

@rycus86, @matthijskooijman Do you know what [ -x "$file"] does. I think its not the same as checking if the "owner" has execution right. I am not sure. Does it also incorporate if the user is in the group and the group has the executbale flag set?

matthijskooijman commented 3 years ago

Do you know what [ -x "$file"] does

man [ says:

  -x FILE
         FILE exists and execute (or search) permission is granted

I suspect this checks whether the current user has effective "x" permissions on the file (because they are the owner and the owner has +x, they are in the owning grou and the group has +x or others have +x)?

gabyx commented 3 years ago

I found a solution: https://github.com/rycus86/githooks/pull/129/commits/53db07b4dfd3137e1f3fa8bbcb9313c682deec45#diff-a5982e9eccefb8a42e5403f0a6324886

I also directly added a new feature to define a runner cmd (first good solution, needs more thoughts) for hookPath by specifying the command in the file hookPath + ".runner if it exists. GetHookRunCmd returns the run command cmd arg1, ... argN in case it is not executable, if its an executable (unix -> access C-function, windows: check ".exe" (first good solution)) it returns nil meaning we can directly execute the file.

Other hook manager configure everything in a yaml/json file (lefthook). The thing about that is, that you need to read the whole config in every hook run, which is a bit inefficient. So I am not starting to go down this path... I stay as closesly to the existing implementation, and make useful adjustments if it proves to be no problems for the tests.

I think first design principle is: Read as less config values and files as you can when executing the runner. If you need to, only read it when you really need to. Currently the following is read always at startup:

Not yet sure: may be we can also leave the file format as it was, to be faster...

If you want more speed: -> Make a service which provide and cache these values for a repository (overkill)

rycus86 commented 3 years ago

I'd suggest adding some timing output for debugging, I don't necessarily think reading config should take that long to be honest, so perhaps don't worry about it for now (especially if you intend to keep the existing tests for the rewrite). I'd keep config and operations the same (just replaced implementation) then once it's ready to be switched over it'll be easier to change config formats and such.

gabyx commented 3 years ago

@matthijskooijman , @rycus86 : Do you know something about that? https://stackoverflow.com/questions/64294818/how-to-get-controlling-terminal-cross-platform-in-go There is also: https://github.com/mattn/go-isatty, but dont know if that helps...

gabyx commented 3 years ago

@rycus86 If you wanna try the prompting some first steps: -> Readme

./build.sh && ./test.sh
rycus86 commented 3 years ago

@matthijskooijman , @rycus86 : Do you know something about that? https://stackoverflow.com/questions/64294818/how-to-get-controlling-terminal-cross-platform-in-go There is also: https://github.com/mattn/go-isatty, but dont know if that helps...

I fought with this a lot, even just targeting Linux, but it's a sh*tshow... πŸ™ˆ See https://github.com/rycus86/ddexec/blob/master/pkg/exec/streams.go -- though I never managed to get it working properly, I still have problems when you pipe input/output, etc.

What do you need this for here specifically?

gabyx commented 3 years ago

Ah thanks for pointing me to: https://github.com/moby/term I need a portable way of making a terminal. On Unix I open /dev/tty. on windows (good luck). Maybe moby/term can make a terminal for us on any platform. Connecting to /dev/tty on Unix and to a new allocated terminal on windows. Dunno ;-) ->Solved...

matthijskooijman commented 3 years ago

@matthijskooijman , @rycus86 : Do you know something about that?

Nope, no experience with getting terminals on Go or cross-platform, sorry.

gabyx commented 3 years ago

Current cd githooks && build.sh && test.sh. Launching all batches in parallel:

πŸ›   Githooks: Install dir set to: '/home/gabyx/.githooks'.
❓ Githooks: This repository wants you to trust all current and
   future hooks without prompting.
   Do you want to allow running every current and future hooks? (yes, No) [y/N]: 
πŸ›   Githooks: 
   - Args: '[]'
   - Repo Path: '/tmp/tmp.PAoqPgLhKM/repo'
   - Repo Hooks: '/tmp/tmp.PAoqPgLhKM/repo/.githooks'
   - Git Dir: '/tmp/tmp.PAoqPgLhKM/repo/.git'
   - Install Dir: '/home/gabyx/.githooks'
   - Hook Path: '/tmp/tmp.PAoqPgLhKM/repo/.git/hooks/pre-commit'
   - Trusted: 'false'
πŸ›   Githooks: Register repo '/tmp/tmp.PAoqPgLhKM/repo/.git'
πŸ›   Githooks: Checksum store contains '0' parsed checksums from '0' files
   and '0' directory search paths.
πŸ›   Githooks: Worktree ignore patterns: 'none' 
πŸ›   Githooks: User ignore patterns: 'none' 
❓ Githooks: New or changed hook found:
   '/tmp/tmp.PAoqPgLhKM/repo/.git/hooks/pre-commit.replaced.githook'
   Do you accept the changes? (Yes, all, no, disable) [Y/a/n/d]: 
πŸ›   Githooks: Executing hook: '/tmp/tmp.PAoqPgLhKM/repo/.git/hooks/pre-commit.replaced.githook'.
hello from old hook
πŸ›   Githooks: Getting hooks in '/tmp/tmp.PAoqPgLhKM/repo/.githooks'
❓ Githooks: New or changed hook found:
   '/tmp/tmp.PAoqPgLhKM/repo/.githooks/pre-commit/gaga'
   Do you accept the changes? (Yes, all, no, disable) [Y/a/n/d]: 
❓ Githooks: New or changed hook found:
   '/tmp/tmp.PAoqPgLhKM/repo/.githooks/pre-commit/monkey'
   Do you accept the changes? (Yes, all, no, disable) [Y/a/n/d]: 
πŸ›   Githooks: Updating all shared hooks
πŸ›   Githooks: Updating shared hook: '/home/gabyx/.githooks/shared/7bd673c497471092a08a455961290d7845de4ef1file:/tmp/tmp.PAoqPgLhKM/shared2.git'
πŸ›   Githooks: Updating shared hook: '/home/gabyx/.githooks/shared/7bd673c497471092a08a455961290d7845de4ef1file:/tmp/tmp.PAoqPgLhKM/shared2.git'
πŸ›   Githooks: Getting hooks in '/home/gabyx/.githooks/shared/7bd673c497471092a08a455961290d7845de4ef1file:/tmp/tmp.PAoqPgLhKM/shared2.git'
❓ Githooks: New or changed hook found:
   '/home/gabyx/.githooks/shared/7bd673c497471092a08a455961290d7845de4ef1file:/tmp/tmp.PAoqPgLhKM/shared2.git/pre-commit'
   Do you accept the changes? (Yes, all, no, disable) [Y/a/n/d]: 
⚠  Githooks: Shared hooks entry:
   'file:///tmp/tmp.PAoqPgLhKM/shared2.git'
   is already listed and will be skipped.
πŸ›   Githooks: Getting hooks in '/tmp/tmp.PAoqPgLhKM/shared1.git'
❓ Githooks: New or changed hook found:
   '/tmp/tmp.PAoqPgLhKM/shared1.git/pre-commit/say-hello'
   Do you accept the changes? (Yes, all, no, disable) [Y/a/n/d]: 
πŸ›   Githooks: Getting hooks in '/tmp/tmp.PAoqPgLhKM/shared2.git'
πŸ›   Githooks: Local Hooks :
    Batch: 0
     - '/tmp/tmp.PAoqPgLhKM/repo/.githooks/pre-commit/gaga'
    Batch: 1
     - '/tmp/tmp.PAoqPgLhKM/repo/.githooks/pre-commit/monkey'

πŸ›   Githooks: Repo Shared Hooks :
    Batch: 0
     - '/home/gabyx/.githooks/shared/7bd673c497471092a08a455961290d7845de4ef1file:/tmp/tmp.PAoqPgLhKM/shared2.git/pre-commit'

πŸ›   Githooks: Local Shared Hooks :
    Batch: 0
     - '/tmp/tmp.PAoqPgLhKM/shared1.git/pre-commit/say-hello'
    Batch: 1
     - '/tmp/tmp.PAoqPgLhKM/shared2.git/pre-commit'

πŸ›   Githooks: Global Shared Hooks: none
πŸ›   Githooks: Create thread pool
πŸ›   Githooks: Launching local hooks ...
hello from repo hook gaga
hello from repo hook monkey
πŸ›   Githooks: Launching repository shared hooks ...
From shared hook 2
πŸ›   Githooks: Launching local shared hooks ...
From shared hook 1
From shared hook 2
πŸ›   Githooks: Launching global shared hooks ...
πŸ›   Githooks: All done.

πŸ›   Githooks: Runner execution time: '4.153850408s'.

The Go solution, gathers all hooks, does the opt-in checks and the executes them in parallel if the shared hook repo are setup in a way (to discuss) such that batches can be made easily.

We could for example support simple subfolders pre-commit/batch-{1...N} or somthing like that to collect all hooks into parallel batches ;-)

By the way: The full execution with default answers with build.sh && test &>Log takes approx 100ms :tada:

gabyx commented 3 years ago

@rycus86: About to mock the runner to prepare it for the tests :crossed_fingers: : The whole thing in Go gets so much easier. With build tags (for mocking etc.) and also error handling. I think the whole runner get so much more maintainable, I just see it while working on it, also when I try to really encapsulate functionality and group concepts which belong togehter in different files/modules. To be continued :-)....

Next ist the fetch&update stuff in the base-template.sh, then it almost should be ready to test :-P (needs some more work)

gabyx commented 3 years ago

With testing so far test-alpine-go.sh: I comply 99% to the implementation before. There are some small nitpicks which I will explain later (see dc69cc5). Tests are passing nicely so far.

gabyx commented 3 years ago

All 116 tests pass (2 skipped, LFS and 120 which only needs install) :tada: See: f179150 for details. There are only minor test fixes.

Questions:

gabyx commented 3 years ago

I am wondering what an approach would be to compile GO on install and deploy it? @rycus86 Do you have any idea?

gabyx commented 3 years ago

@rycus86 If you are interested, I tried to summarize what has been done and what is new and what has bee deprecated while I was building the runner executable. Please see the first post in this PR. Its from the githooks/Readme.md. Thanks for inputs.

rycus86 commented 3 years ago

I am wondering what an approach would be to compile GO on install and deploy it? @rycus86 Do you have any idea?

I have some examples on cross-compiling Go on Travis we could potentially use here, then the installation could be a shell script fetching the platform-appropriate binary with curl or wget, then perhaps delegating the install procedure to it (maybe it needs to fetch multiple binaries afterwards?)

Glad to see you're making good progress with this! I haven't had a chance to look at this properly yet, but I see you made some changes and have some questions you summarized above. I agree that perhaps this GitHub PR is not the best medium to discuss details, do you have any suggestions what to use instead? You mentioned discord, which I haven't used before,but could try. I used Gitter once, or perhaps we could look at Slack, or anything else you could suggest.

gabyx commented 3 years ago

Jeah :) ehm discord is shitty somehow. Maybe slack where we have proper threads. Maybe we could also chat on Teams once in a while :). Ehm. ah jeah I didnt thought about getting the binaries from travis. As the build takes not even a second, and also when you use another branch and such building from source would be really easy, however you need a working go platform hmmm... Just thinking because of our different branch in the company :)

Von meinem iPhone gesendet

Am 31.10.2020 um 00:18 schrieb Viktor Adam notifications@github.com:

I am wondering what an approach would be to compile GO on install and deploy it? @rycus86 Do you have any idea?

I have some examples on cross-compiling Go on Travis we could potentially use here, then the installation could be a shell script fetching the platform-appropriate binary with curl or wget, then perhaps delegating the install procedure to it (maybe it needs to fetch multiple binaries afterwards?)

Glad to see you're making good progress with this! I haven't had a chance to look at this properly yet, but I see you made some changes and have some questions you summarized above. I agree that perhaps this GitHub PR is not the best medium to discuss details, do you have any suggestions what to use instead? You mentioned discord, which I haven't used before,but could try. I used Gitter once, or perhaps we could look at Slack, or anything else you could suggest.

β€” You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

rycus86 commented 3 years ago

Can those differences be handled by different configuration rather than different binary? E.g. do they have to be compile-time differences?

For the comms, let's try Slack then, I have a workspace I use for automation already, I can add a channel there. Send me your email to gabyx[at]viktoradam[dot]net and I'll send you an invite.

matthijskooijman commented 3 years ago

I agree that perhaps this GitHub PR is not the best medium to discuss details, do you have any suggestions what to use instead?

https://element.io/ is also nice, it's a client for the open, federated "matrix" protocol with support for encryption. Sort of IRC, but modernized with clients for web, mobile and desktop.

gabyx commented 3 years ago

As replacing( rm + create) binaries while executing works (linux, win hopefully as well). I think there is nothing which stops us from having the same impl. in go for install.sh. I just want to think first a bit more about what part does what during install and what are the odds when it crashes ;). how do we revert without ending up in shitload of troubles ;)

Von meinem iPhone gesendet

Am 31.10.2020 um 00:18 schrieb Viktor Adam notifications@github.com:

I am wondering what an approach would be to compile GO on install and deploy it? @rycus86 Do you have any idea?

I have some examples on cross-compiling Go on Travis we could potentially use here, then the installation could be a shell script fetching the platform-appropriate binary with curl or wget, then perhaps delegating the install procedure to it (maybe it needs to fetch multiple binaries afterwards?)

Glad to see you're making good progress with this! I haven't had a chance to look at this properly yet, but I see you made some changes and have some questions you summarized above. I agree that perhaps this GitHub PR is not the best medium to discuss details, do you have any suggestions what to use instead? You mentioned discord, which I haven't used before,but could try. I used Gitter once, or perhaps we could look at Slack, or anything else you could suggest.

β€” You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or unsubscribe.

gabyx commented 3 years ago

Can those differences be handled by different configuration rather than different binary? E.g. do they have to be compile-time differences?

I think yes :). Only configuration no binary differences =). The thing is that I could controll when an update happens for company installation by a different branch. Dont know how this is possible with binary distributions and downloading... I think not so hard to do, just configure with a different link ;-), so basically no release folder is necessary. lets continue the dicsussion on Slack.

gabyx commented 3 years ago

@rycus86 Can you not connect this repo with slack somehow? ;)

rycus86 commented 3 years ago

@rycus86 Can you not connect this repo with slack somehow? ;)

Hm, I'm not sure that's a thing to be honest, but happy to set it up if you find out how to do it. (a quick Google search didn't surface anything useful)

gabyx commented 3 years ago

Can you try this? https://slack.github.com/ And then somehow make a channel for this project? Or do we need a workspace, ah I dont get slack lool is it so complicated ;-)

gabyx commented 3 years ago

Otherwise just connect wie gitter and we talk there ;-)

gabyx commented 3 years ago

Gitter seems easier to handle I think ;-)

gabyx commented 3 years ago

Discussion is on: https://gitter.im/rycus86-githooks/community

gabyx commented 3 years ago

Runner/Installer/Uninstaller finished, tests pass: :partying_face: Here a Debug Session inside the .devcontainer in VS Code:

https://user-images.githubusercontent.com/647437/103219402-e5032580-491d-11eb-94d5-ef5f6b3459fb.mp4

gabyx commented 3 years ago

First release with goreleaser. Works like a charm and ist dead easy. https://github.com/gabyx/githooks/releases/tag/v1.0.0-rc1

Not to be used! Just testing...

gabyx commented 3 years ago

@matthijskooijman , @rycus86 : The current GO implementation is pretty awesome. Now we also have the possibility to properly ignore/activate or untrust/trust hooks by matching a hook's namespace path. Thats also how the ignore mechanism is implemented. Furthermore: The checksum store supports in-repo checksum dir (similar to .git/objects) or a global checksum directory which is used.

See the autogenerated doc: https://github.com/gabyx/githooks/blob/feature/go-refactoring/docs/git_hooks_trust_hooks.md

rycus86 commented 3 years ago

Hey @gabyx πŸ‘‹

It's awesome to see the constant rate of commits as you make progress on this! I haven't had a chance to try out your implementation yet or to look at the new Go code, but let me know when it's in a good state and people can install it, and I'm happy to put up some signposting here towards your repo as an alternative/improved version.

I don't think I'll have too much time in the near future to do major things with this project, apart from maybe an odd bugfix or easy change here and there, so I think you should treat your fork as the main source for developing and distributing the Go version.

gabyx commented 3 years ago

@rycus86 : Ah, nice to here from you. Yeah, I will let you know when the impl. is in a good state, meaning when everything works. I finish the CLI + then overlook the update procedure again (its probably still too complicated...) + reafactoring to get the code constantly in a good shape, I think it is already quite good, but needs some tweaks at the end for sure, I have maintainability and a proper written software in mind, not a GO spagetti mess...

(if we still need the release clone, I am not sure yet if it is really needed, the nice thing to have it together with an associated deploy URL (also in a yaml config inside the branchr e.g.), it guides us for the update procedure, as we anyway write a tool for Git, we can directly use the tags on the specified release branch (with version tags which have "dont-skip" -> git interprete-trailers) to guide the update procedure (so far implemented). Or we only use a deploy url (no release clone), where we then use Gitea/Github API code to get the latest version (version "dont-skip" might be harder get right... which is a feature which might come handy for updates which need to be executed...) and cant be skipped... So my thought was,

gabyx commented 3 years ago

@rycus86 Its finished to give it a first try: Remember: It wont work properly (removed and renamed things) with your legacy installation, to not destroy your setup: Use the checkout, and open den Docker .devcontainer in VS Code and work in the terminal directly.

cd githooks
build.sh --no-debug
bin/installer \       
        --clone-url="https://github.com/gabyx/githooks.git" \
        --clone-branch="feature/go-refactoring" \
        --build-from-source

I am currently using it in the developement already.

https://github.com/gabyx/githooks/releases/tag/v1.0.0-rc1

I will now continue cleanup and rethink the update procedure. (is the release clone really needed ?)...

gabyx commented 3 years ago

@rycus86 , @matthijskooijman Anybody working in security or knows how to validate a download? Can I just compare the githooks.checksums here: https://github.com/gabyx/githooks/releases/tag/v1.0.0-rc3 there is also a signature file githooks.checksums.sig which is the gpg signed file (with my GPG) of githooks.checksums (I think). I am wondering if we need to get the checksums over another tunnel? I mean it would access the same API for the checksums and the assert (zip, tar.gz) which is bad?? I dont know... Maybe the user can provide the GPG public key along with the download specification (gitea/github... what ever) to check the .sig file if its valid or so? optionaly bypassing this check (during install), default "no".

Thanks for the input :-)

rycus86 commented 3 years ago

@rycus86 , @matthijskooijman Anybody working in security or knows how to validate a download? Can I just compare the githooks.checksums here: https://github.com/gabyx/githooks/releases/tag/v1.0.0-rc3 there is also a signature file githooks.checksums.sig which is the gpg signed file (with my GPG) of githooks.checksums (I think). I am wondering if we need to get the checksums over another tunnel? I mean it would access the same API for the checksums and the assert (zip, tar.gz) which is bad?? I dont know... Maybe the user can provide the GPG public key along with the download specification (gitea/github... what ever) to check the .sig file if its valid or so? optionaly bypassing this check (during install), default "no".

Thanks for the input :-)

I'm NOT very much in security, but here's an example I did: https://github.com/rycus86/ddexec/blob/master/install.sh I saw other places (like get.docker.com) also just using curl -fsSL to download the verification keys/hashes, I think that's probably OK. What's your concern in particular?

gabyx commented 3 years ago

ok thanks, the concern I had was: anybody can provide an executable with correct hashes. if the site gets hacked, you install something which is perfectly valid and probably hacked... -> πŸ’£ . Can that be done with signature verifacation? So actually its pretty easy =).

So everything we need to be secure is already provided with the deploy assets =) awesommmme :-)

gabyx commented 3 years ago

@rycus86 Windows tests are still to go: I am testing now finally with docker and windowsserver. Its utter crap but better than on travis to control the behavior. I once more soooo hate this OS, its unbelievable what an uttter BS it is... The test failures so far will also be bugs/windows crap and hopefully other unexpected stuff...

BR

gabyx commented 3 years ago

@rycus86 , @matthijskooijman : Almost finished: Check: https://github.com/gabyx/githooks

rycus86 commented 3 years ago

@rycus86 , @matthijskooijman : Almost finished: Check: https://github.com/gabyx/githooks

Nice one! I ought to check it out at some point, will try to find some time soon.

gabyx commented 3 years ago

@rycus86:

I can finally close this. It was huge work, good fun, grulesome migration with the test (esp. windows...), but I think its in a good shape. There is stilll some stuff missing out for the various deploy scenarios. Github works well so far as I am testing the updates at the moment. The release process is a breeece -> Just push a tag -> Github Action with Goreleaser will do the rest :-) :+1: Its signed too! Verified during download of course!

I added you as a collaborator (feel free =).

You should probably not push into master right now, rather open PRs if you want, I am still cleaning up a bit. I will definitely protect "main"-branch, and each PR needs a review before we can merge it.

It would be cool if you could post a bold sign on the front page of the old implementation redirecting to the new one :-). Maybe better when I am confident enough to release the beast ;-).

Ok sad that the Github stars do not get transferred ;-), but anyway, your vision and features are the right implementation I think for a hook manager, thats why I like it. Its powerful, but still fully configurable.

A review would have been nice, but jeah everybody has little time, so far I kept to my own principles -> clean and as proper as possible... maybe you could read through the Readme.md and questions, english corrections and suggestions may pop up, which I can still take up and implement, that would help :-)

-> https://github.com/gabyx/githooks

gabyx commented 3 years ago

Ah of course: We need covarge, that another ticket I need to address...

rycus86 commented 3 years ago

Awesome news @gabyx well done! I'll try to fix up the README here and open a PR on yours for spelling things today if I can get to it.

Thanks a lot for taking on the evolution of this project!