Chatie / git-scripts

Git Hooks Integration for Chatie Projects
https://npmjs.com/package/@chatie/git-scripts
Apache License 2.0
1 stars 4 forks source link

Show a hint for how to disable pre-push hook in the error message #18

Open huan opened 2 years ago

huan commented 2 years ago

RxJS has a very nice error message to let users know how to skip the pre-push hook, so do us.

husky > commit-msg hook failed (add --no-verify to bypass)

$ git commit -m 'fixx(fromEvent): clean fromEvent.ts code to make sure its clean'
husky > pre-commit (node v16.3.0)
✔ Preparing...
✔ Running tasks...
✔ Applying modifications...
✔ Cleaning up... 
husky > commit-msg (node v16.3.0)
INVALID COMMIT MSG: "fixx" is not allowed type ! Valid types are: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
fixx(fromEvent): clean fromEvent.ts code to make sure its clean
husky > commit-msg hook failed (add --no-verify to bypass)

CC @lucifer1004

binsee commented 2 years ago

I read the code of git, and the running process of git push hook is roughly as follows:


int cmd_push(int argc, const char **argv, const char *prefix){
  // ...
    rc = do_push(flags, push_options, remote);
  // ...
  return rc;
}

int do_push(int flags, const struct string_list *push_options, struct remote *remote) {
  // ...
  if (push_with_options(transport, push_refspec, flags))
    errs++;
  // ...
    return !!errs;
}

int push_with_options(){
  // ...
    err = transport_push(the_repository, transport, rs, flags, &reject_reasons);
  // ...
    if (err != 0) {
        fprintf(stderr, "%s", push_get_color(PUSH_COLOR_ERROR));
        error(_("failed to push some refs to '%s'"), anon_url);
        fprintf(stderr, "%s", push_get_color(PUSH_COLOR_RESET));
    }
  // ...
}

int transport_push(){
  // ...
  if (run_pre_push_hook(transport, remote_refs))
          return -1;
  // push  submodules
  // ...
}

int run_pre_push_hook() {
  // run hook
  return ret;
}

In push_with_options (),if return is err for call transport_push(), print error msg. So, we want do not show error msg, must return 0 for call transport_push(), and must return 0 for call run_pre_push_hook(), and return 0 for run pre-push script file.

I think we can change git push command args, like run git push --dry-run {retome} {branch}, run git push {retome} {branch} in pre-push script file, and return 0 in pre-push script file.

pre-push This hook is called by git-push[1] and can be used to prevent a push from taking place.

The design purpose of the pre-push hook is to choose whether to block the push, not to change the push content, so using the --dry-run parameter is more suitable for our scenario.

huan commented 2 years ago

The git push hook sometimes will confuse our first-time contributors to the Wechaty community.

So I think when there's any push action has been blocked, we can show a message like the following:

The push has been blocked because XXX ...
If you think this should not be blocked, you can skip the push hook by setting the below environment variable when running git push:
   $ NO_HOOK=1 git push

Learn more from our issue: https://github.com/Chatie/git-scripts/issues/18
binsee commented 2 years ago
test > git checkout -b test2
Switched to a new branch 'test2'
test2 > git push --dry-run

> @chatie/git-scripts@0.7.1 lint
> npm run lint:es && npm run lint:ts

> @chatie/git-scripts@0.7.1 lint:es
> eslint bin/**/*.ts src/**/*.ts tests/**/*.ts

> @chatie/git-scripts@0.7.1 lint:ts
> tsc --noEmit

v0.7.2
remote: 
remote: Create a pull request for 'test2' on GitHub by visiting:        
remote:      https://github.com/binsee/chatie-git-scripts/pull/new/test2        
remote: 
To github.com:binsee/chatie-git-scripts.git
 * [new branch]      test2 -> test2

____ _ _        ____            _
/ ___(_) |_     |  _ \ _   _ ___| |__
| |  _| | __|    | |_) | | | / __| '_ \
| |_| | | |_     |  __/| |_| \__ \ | | |
\____|_|\__|    |_|    \__,_|___/_| |_|

____                              _ _
/ ___| _   _  ___ ___ ___  ___  __| | |
\___ \| | | |/ __/ __/ _ \/ _ \/ _^ | |
___) | |_| | (_| (_|  __/  __/ (_| |_|
|____/ \__,_|\___\___\___|\___|\__,_(_)

 ### Npm version bumped and pushed by inner push inside hook pre-push ###"
 -- vvvvvv outer push will be canceled, don't worry, not bug :) vvvvvv --"

Failed to exec pre-push hook script
error: failed to push some refs to 'git@github.com:binsee/chatie-git-scripts.git'

test2 > git push --dry-run
Everything up-to-date

Now git push will not be blocked, because in the pre-push hook, we append the refs parameter to the new git push, so the push should always succeed.

What bothers everyone is that when you push for the first time, because there is no remote refs information, it is blocked, so you need to use NO_HOOK=1 to create refs information. And now the mechanism has been changed, so the problem doesn't occur anymore.

The only problem is that the outer git push cannot really push. In the hook, the version will be bumped first, which will add a new commit id, and running git push in the child process will push the latest commit id. At this time, the outer git push where the hook is located is still the old commit id, and an error will be reported when pushing to the server.

So the hook must return non-zero to prevent the outer git push from actually pushing, but this will throw an error. This prompt cannot be skipped unless the code of the git project is modified.

We can use the --dry-run parameter externally to make the outer git push skip the actual push, and then the hook returns 0.

Also we need a unique return value, when this return value is found in hook/pre-push, instead of showing an error message, show Please ignore the next line of red warning from git.

binsee commented 2 years ago

We now have a better reading experience.

There is only one question left.

I hope git can add new features, when the hook exit code is a specific value, no error is displayed, and it will immediately use 0 as the exit code and end the run.

huan commented 2 years ago

Awesome, the output message layout has been fixed after merging your PR.

Thank you very much for helping the community to improve the developing experiences!

binsee commented 2 years ago

Hope to release a new version to npm soon.

I plan to upgrade this dependency for multiple projects in the community to solve the push blocking problem.

huan commented 2 years ago

I have just released the new version.

Sorry for I have forgotten this repo has no DevOps configured.

Please let me know if there's anything I missed.

Thanks!