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

Split skip_output.execution into two options #473

Closed
hustcer opened this issue Apr 26, 2023 · 27 comments · Fixed by #475, #476 or #484
Closed

Split skip_output.execution into two options #473

hustcer opened this issue Apr 26, 2023 · 27 comments · Fixed by #475, #476 or #484
Labels
feature request A new lefthook feature description

Comments

@hustcer
Copy link

hustcer commented Apr 26, 2023

⚡ Summary

# lefthook.yml

skip_output:
  - execution  # Skips printing successfully executed commands and their output (but still prints failed executions)

As the config comment says skip_output.execution will skip printing successfully executed commands and their output, can this config option be split into two parts, something like:

# lefthook.yml

skip_output:
  - execution      # Skips printing successfully executed commands
  - cmd_output     # Skips printing successfully executed commands' output (but still prints failed executions)

Value

I don't want to know what has been executed, as if I saw the output I will know what's happenning.

@hustcer hustcer added the feature request A new lefthook feature description label Apr 26, 2023
@hustcer
Copy link
Author

hustcer commented Apr 27, 2023

I think this option will allow the user to decide by themselves what can be output and will make this tool more user friendly

@mrexox
Copy link
Member

mrexox commented Apr 27, 2023

Hey! Thank you for the feature request. I decided to slightly change the option - execution_out. Does it feel OK for you?

@hustcer
Copy link
Author

hustcer commented Apr 27, 2023

Yes, Great, Thanks for the quick fix

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

It's weird that it does not work as expected after upgrade to the latest version:

min_version: 1.3.11
color: false
no_tty: false
skip_output:
  - meta              # Skips lefthook version printing
  - skips             # Skips "skip" printing (i.e. no files matched)
  - summary           # Skips summary block (successful and failed steps) printing
  - success           # Skips successful steps printing
  - failure           # Skips failed steps printing
  - execution         # Skips printing successfully executed commands
  # - execution_out   # Skips printing successfully executed commands' output (but still prints failed executions)
pre-push:
  follow: true # need to use follow to get stdin from git push
  scripts:
    'sync.sh':
      runner: sh

For me when I set skip execution then the execution output be skipped too.
When I disable skip execution and enable skip execution_out everything works as 1.3.10 with skip execution disabled: the executed commands and it's output been displayed

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

Hmm I guess I haven't handled the case when you use follow/interactive option. Let me fix that.

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

Hi, thanks for the new release, I have just tried 1.3.12, but still no luck.

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

I've tried your config, and it worked for me:

skip_output:
  # - meta              # Skips lefthook version printing
  # - skips             # Skips "skip" printing (i.e. no files matched)
  # - summary           # Skips summary block (successful and failed steps) printing
  # - success           # Skips successful steps printing
  # - failure           # Skips failed steps printing
  # - execution         # Skips printing successfully executed commands
  - execution_out   # Skips printing successfully executed commands' output (but still prints failed executions)
pre-push:
  follow: true # need to use follow to get stdin from git push
  scripts:
    'sync.sh':
      runner: sh
$ cat .lefthook/pre-push/sync.sh
echo waiting...
sleep 1
echo done
sleep 1

$ lefthook run pre-push
Lefthook v1.3.12
RUNNING HOOK: pre-push

  EXECUTE >  sync.sh

SUMMARY: (done in 2.02 seconds)
✔️  sync.sh

What do you see if you remove all skip_output settings?

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

My sync.sh:

#!/bin/sh

while read -t 2 -s local_ref local_oid remote_ref remote_oid
do
  echo waiting...
  echo $local_ref
  sleep 1
  echo done
  sleep 1
done

exit 0

Config:

min_version: 1.3.12
color: false
no_tty: false
# skip_output:
#   - meta              # Skips lefthook version printing
#   - skips             # Skips "skip" printing (i.e. no files matched)
#   - summary           # Skips summary block (successful and failed steps) printing
#   - success           # Skips successful steps printing
#   - failure           # Skips failed steps printing
#   - execution         # Skips printing successfully executed commands
  # - execution_out   # Skips printing successfully executed commands' output (but still prints failed executions)
pre-push:
  follow: true        # need to use follow to get stdin from git push
  scripts:
    'sync.sh':
      runner: sh

Output:

Lefthook v1.3.12
RUNNING HOOK: pre-push

  EXECUTE >  sync.sh
refs/heads/feature/sync 98c0a7490b34d901573ce709dcfe612beec4854c refs/heads/feature/sync 5498b11eb3a7ea2a17ef215914591f3ef5d4f396
waiting...
refs/heads/feature/sync
done

SUMMARY: (done in 4.04 seconds)
✔️  sync.sh
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 332 bytes | 166.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
To https://...............

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

With this config:

min_version: 1.3.12
color: false
no_tty: false
skip_output:
  - meta              # Skips lefthook version printing
  - skips             # Skips "skip" printing (i.e. no files matched)
  - summary           # Skips summary block (successful and failed steps) printing
  - success           # Skips successful steps printing
  - failure           # Skips failed steps printing
  - execution         # Skips printing successfully executed commands
  # - execution_out   # Skips printing successfully executed commands' output (but still prints failed executions)
pre-push:
  follow: true        # need to use follow to get stdin from git push
  scripts:
    'sync.sh':
      runner: sh

Got output:

Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 322 bytes | 80.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
To https://................
   98c0a749..6fd2c285  feature/sync -> feature/sync

BTW: I'm using git push to do a real push

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

Ok, and what do you see with this config?

min_version: 1.3.12
color: false
no_tty: false
skip_output:
  - meta              # Skips lefthook version printing
  - skips             # Skips "skip" printing (i.e. no files matched)
  - summary           # Skips summary block (successful and failed steps) printing
  - success           # Skips successful steps printing
  - failure           # Skips failed steps printing
  # - execution         # Skips printing successfully executed commands
  - execution_out   # Skips printing successfully executed commands' output (but still prints failed executions)
pre-push:
  follow: true        # need to use follow to get stdin from git push
  scripts:
    'sync.sh':
      runner: sh

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

  EXECUTE >  sync.sh
refs/heads/feature/sync 8fb4420dd8818b68115df7abd121ba3b2784b2fc refs/heads/feature/sync 6fd2c285dbd6513c85874fc935ac137abfcce86c
waiting...
refs/heads/feature/sync
done
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 294 bytes | 147.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
To https://.....................
   6fd2c285..8fb4420d  feature/sync -> feature/sync

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

Well, I can't reproduce that, for my git push output is being skipped correctly.

Can you remove skip_output settings and check ENV-var approach?

LEFTHOOK_QUIET=execution_out git push

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

Sure.
(feature/sync) $> LEFTHOOK_QUIET=execution_out git push

Lefthook v1.3.12
RUNNING HOOK: pre-push

  EXECUTE >  sync.sh
refs/heads/feature/sync 4d9067adae1709abf48c9598c35afbdc8164ab4c refs/heads/feature/sync 8fb4420dd8818b68115df7abd121ba3b2784b2fc
waiting...
refs/heads/feature/sync
done

SUMMARY: (done in 4.04 seconds)
✔️  sync.sh
Enumerating objects: 5, done.
Counting objects: 100% (5/5), done.
Delta compression using up to 8 threads
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 333 bytes | 166.00 KiB/s, done.
Total 3 (delta 2), reused 0 (delta 0), pack-reused 0
To https://................
   8fb4420d..4d9067ad  feature/sync -> feature/sync

BTW: My git version: 2.40.1. macOS: 13.3.1 (22E261)

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

Ok, this thing drive me crazy :)

I have Arch Linux, git version: 2.40.1

Try removing follow option. Does it change anything?

BTW, follow option doesn't affect STDIN, it is only for STDOUT to appear without a delay

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

Ok, this thing drive me crazy :)

I have Arch Linux, git version: 2.40.1

Try removing follow option. Does it change anything?

BTW, follow option doesn't affect STDIN, it is only for STDOUT to appear without a delay

Still no luck yet. I think it's probably an issue specific to macOS. No worries, I can use it without skip execution and execution_out.
Very much appreciated.

@mrexox
Copy link
Member

mrexox commented Apr 28, 2023

Ok. But if you have time, I'd be glad to check a few possible reasons of this behavior:

  • Maybe it forwards STDOUT to STDERR. What if you do git push 2>&1
  • Or try interactive: true option for the script. It might be pty-related issue. With interactive: true we don't use pty lib.

Anyway thank you for bringing this feature idea. It's a pity it doesn't work correctly for you 😕

@hustcer
Copy link
Author

hustcer commented Apr 28, 2023

  • Maybe it forwards STDOUT to STDERR. What if you do git push 2>&1
  • Or try interactive: true option for the script. It might be pty-related issue. With interactive: true we don't use pty lib.

I have just tried these two ways, and they all make no difference, It's really weird.
And I'm always happy to do more tests.

@hustcer
Copy link
Author

hustcer commented May 11, 2023

Hi, Any plan to fix the skip issue for macOS in the future version? Thanks

@mrexox mrexox reopened this May 11, 2023
@mrexox
Copy link
Member

mrexox commented May 11, 2023

I put this issue on my radar. I will come with the details next week

@mrexox
Copy link
Member

mrexox commented May 18, 2023

Hi @hustcer! I have tried to reproduce this on my macOS machine but I had no luck. I have no idea why this might happen. Do you have an example repo with this config that I can fork and test on?

@hustcer
Copy link
Author

hustcer commented May 18, 2023

@mrexox Hi, please try https://github.com/hustcer/setup-nu/tree/feature/lefthook I have just added a demo pre-push hook on feature/lefthook branch, for the config: https://github.com/hustcer/setup-nu/blob/d631c0697e03cc5b5fbce80ef5dfa0048456251e/lefthook.yml#LL15C25-L15C137 I expect "Skips printing successfully executed commands, but don not skip execution_out, however no output will be printed"
Hope it helps

@mrexox
Copy link
Member

mrexox commented May 18, 2023

I've created a PR with the changes to config that should work (nothing echoes to stdout for me on git push). Could you check it, please? Also I'm preparing a new release with the fix for interactive option - I noticed that stdout is being printed despite the execution_out setting.

@hustcer
Copy link
Author

hustcer commented May 18, 2023

THANKS for the quick reply I have left my mac laptop at office, I will give it a try tomorrow

@hustcer
Copy link
Author

hustcer commented May 19, 2023

I've created a PR with the changes to config that should work (nothing echoes to stdout for me on git push). Could you check it, please?

@mrexox Hi, I have just tested your PR, it does not work as expected, for this config: https://github.com/hustcer/setup-nu/blob/feature/lefthook/lefthook.yml, I don't want to see EXECUTE > sync.sh be outputted, and I want to see all the echos output from sync.sh(but they didn't), that's why I added execution to skip_output and commented out execution_out

@mrexox
Copy link
Member

mrexox commented May 19, 2023

Oh, I see, so you want to see something like this, right?

$ git push fork --force


refs/heads/feature/lefthook b45ceab5227c2d645255a30ad895b34cc0d51875 refs/heads/feature/lefthook 81d2b27a4b09f0340ae7e987229f9221890ef027
Output from sync.sh in pre push hook:
refs/heads/feature/lefthook
b45ceab5227c2d645255a30ad895b34cc0d51875
refs/heads/feature/lefthook
81d2b27a4b09f0340ae7e987229f9221890ef027
----------------------------------

Enumerating objects: 8, done.
Counting objects: 100% (8/8), done.
Delta compression using up to 8 threads
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 1.90 KiB | 1.90 MiB/s, done.
Total 6 (delta 4), reused 0 (delta 0), pack-reused 0
remote: Resolving deltas: 100% (4/4), completed with 2 local objects.
To github.com:mrexox/setup-nu.git
 + 81d2b27...b45ceab feature/lefthook -> feature/lefthook (forced update)

@hustcer
Copy link
Author

hustcer commented May 19, 2023

Oh, I see, so you want to see something like this, right?

Yeah, that's it. It's better not print refs/heads/feature/lefthook b45ceab5227c2d645255a30ad895b34cc0d51875 refs/heads/feature/lefthook 81d2b27a4b09f0340ae7e987229f9221890ef027, I think that's part of execution

@mrexox
Copy link
Member

mrexox commented May 19, 2023

Hm, I'm not quite sure about why refs/heads/feature/lefthook b45ceab5227c2d645255a30ad895b34cc0d51875 refs/heads/feature/lefthook 81d2b27a4b09f0340ae7e987229f9221890ef027 gets printed but this is not lefthook-generated output, so it's from the script.

I think I got you, I'll provide an option execution_info that will skip printing EXECUTE > ... lines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A new lefthook feature description
Projects
None yet
2 participants