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

Cannot kill git pre-commit hook while lingui command is running #1833

Closed
1 of 3 tasks
datafoo opened this issue Jan 11, 2024 · 4 comments
Closed
1 of 3 tasks

Cannot kill git pre-commit hook while lingui command is running #1833

datafoo opened this issue Jan 11, 2024 · 4 comments

Comments

@datafoo
Copy link
Contributor

datafoo commented Jan 11, 2024

Describe the bug
It is not possible to stop a git pre-commit hook after lingui extract --clean has started, even after lingui extract --clean has finished.

To Reproduce
In one of my project I have the following git pre-commit hook (.git/hooks/pre-commit):

#!/usr/bin/env bash
#
# Git pre-commit hook

RED='\033[0;31m'
GREEN='\033[0;32m'
NC='\033[0m'

message()
{
  MESSAGE="$1"
  COLOR="$2"
  echo -e "${COLOR}${MESSAGE}${NC}"
}
error()
{
  message "$1" "$RED";
  exit 1
}
success() { message "$1" "$GREEN" ; }

cd "$(git rev-parse --show-toplevel)" || error "Cannot change working directory to root of Git repository!"

npm --prefix assets run extract
./assets/js/locales/normalize.sh
if npm --prefix assets run compile; then
  success "The frontend code messages have been translated."
else
  error "Commit failed due to frontend code missing translations!"
fi

if npm --prefix assets run prettier; then
  success "The frontend code is properly formatted."
else
  error "Commit failed due to frontend code formatting issues!"
fi

# ...
# several other tests

My package.json contains:

{
  ...
  "scripts": {
    "extract": "lingui extract --clean ",
    ...
  },
  ...
}

After the hook script starts executing npm --prefix assets run extract, then trying to kill the process with CTRL+C does not do much as the script keeps running. I am forced to let the whole script finish or close the terminal tab which is not practical.

Expected behavior
CTRL+C should stop the hook script.

Additional context
Add any other context about the problem here.

  • jsLingui version lingui --version: 4.5.0
  • Babel version npm list @babel/core: 7.23.2
  • Macro support:
  • I'm using SWC with @lingui/swc-plugin
  • I'm using Babel with babel-macro-plugin
  • I'm not using macro
  • Your Babel config (e.g. .babelrc) or framework you use (Create React App, NextJs, Vite)
    .babelrc:
    {
      "presets": ["@babel/preset-env", "@babel/preset-react"],
      "plugins": ["macros"]
    }
@andrii-bodnar
Copy link
Contributor

andrii-bodnar commented Jan 11, 2024

You can try to catch signals like SIGINT (CTRL+C) in the main process and kill the child process:

#!/bin/bash

trap "kill -SIGINT $lingui_pid" SIGINT

# Start the extract process, storing its PID
npm --prefix assets run extract &
lingui_pid=$!

The trap command at the start of your script will kill the lingui process ID stored in the $lingui_pid variable on SIGINT signal. (https://www.linuxjournal.com/content/bash-trap-command)

P.S. Didn't test it, so I'm not sure if it's going to work with git hooks.

@datafoo
Copy link
Contributor Author

datafoo commented Jan 12, 2024

Thank you @andrii-bodnar. I learnt about trap but this does not solve my issue entirely. Also the I cannot run the command in the background with & lingui_pid=$! because the commands need to be executed sequentially.

After more tests, I need to add some corrections to my issue.

Both lingui commands are problematic:

  • npm --prefix assets run extract (lingui extract --clean)
  • npm --prefix assets run compile (lingui compile --strict)

The problem happens if I enter CTRL+C while one of these commands are running.

  • If I am patient enough to wait that both lingui commands are done, then CTRL+C successfully terminates the script.
  • If I enter CTRL+C while one of these commands are running, it will stop the corresponding command but the rest of the script will continue and any further CTRL+C will not terminate the rest of the script.

I have been able to understand a bit more when running the script directly (by entering ./.git/hooks/pre-commit rather than git hook run pre-commit). In that case any lingui command will steal the CTRL+C for itself. So I can stop the hook script by entering CTRL+C up to 3 times: the first to stop lingui extract --clean, second to stop lingui compile --strict and the last to stop the whole hook script. If I let a lingui command finish, then it is one less CTRL+C to enter. So it seems that CTRL+C is "stolen" by the lingui commands.

If I run the hook script normally with git (e.g. git hook run pre-commit), then entering CTRL+C while a lingui command is running will stop that lingui command, but the rest of the script continues and the terminal prompt is displayed again so the next CTRL+C is running in the prompt and does nothing (it does not send the SIGINT signal to the hook script I guess).

Lastly, I noticed that adding trap "exit 1" SIGINT in my script help a lot:

#!/usr/bin/env bash
#
# Git pre-commit hook

RED='\033[0;31m'
GREEN='\033[0;32m'
NC='\033[0m'

message()
{
  MESSAGE="$1"
  COLOR="$2"
  echo -e "${COLOR}${MESSAGE}${NC}"
}
error()
{
  message "$1" "$RED";
  exit 1
}
success() { message "$1" "$GREEN" ; }

cd "$(git rev-parse --show-toplevel)" || error "Cannot change working directory to root of Git repository!"

trap "exit 1" SIGINT

npm --prefix assets run extract
./assets/js/locales/normalize.sh
if npm --prefix assets run compile; then
  success "The frontend code messages have been translated."
else
  error "Commit failed due to frontend code missing translations!"
fi

if npm --prefix assets run prettier; then
  success "The frontend code is properly formatted."
else
  error "Commit failed due to frontend code formatting issues!"
fi

if npm --prefix assets run lint; then
  success "The frontend code meets the quality requirements."
else
  error "Commit failed due to frontend code quality issues!"
fi

if npm --prefix assets test; then
  success "The frontend code tests passed."
else
  error "Commit failed due to frontend code test failures!"
fi

With that modification:

  • when running with ./.git/hooks/pre-commit: a single CTRL+C stops the whole script whether I enter it when a lingui command is running or not.
  • when running with git hook run pre-commit: a single CTRL+C stops the whole script whether I enter it when a lingui command is running or not but if CTRL+C is entered while a lingui command is running then there is little problem with the terminal prompt. I need to press Enter or CTLR+C again to get back to normal.

What is happening with the lingui commands? How can we solve this properly?

@datafoo datafoo changed the title Cannot kill git pre-commit hook after lingui extract --clean has started Cannot kill git pre-commit hook while lingui command is running Jan 12, 2024
@timofei-iatsenko
Copy link
Collaborator

Thanks for detailed investigation. Actually, lungui uses under the hood commander package and don't do anything extra to trap interruption signals whatsoever.

So it's rather regular JS script which is run by node.js. It doesn't spawn it's own child processes or call external tools. So i assume the current behaviour is just a default behaviour for long running scripts in node.

@datafoo
Copy link
Contributor Author

datafoo commented Jan 12, 2024

My .git/hooks/pre-commit is exactly:

#!/usr/bin/env bash
#
# Git pre-commit hook

RED='\033[0;31m'
GREEN='\033[0;32m'
NC='\033[0m'

message()
{
  MESSAGE="$1"
  COLOR="$2"
  echo -e "${COLOR}${MESSAGE}${NC}"
}
error()
{
  message "$1" "$RED";
  exit 1
}
success() { message "$1" "$GREEN" ; }

cd "$(git rev-parse --show-toplevel)" || error "Cannot change working directory to root of Git repository!"

npm --prefix assets run extract
./assets/js/locales/normalize.sh
if npm --prefix assets run compile; then
  success "The frontend code messages have been translated."
else
  error "Commit failed due to frontend code missing translations!"
fi

if npm --prefix assets run prettier; then
  success "The frontend code is properly formatted."
else
  error "Commit failed due to frontend code formatting issues!"
fi

if npm --prefix assets run lint; then
  success "The frontend code meets the quality requirements."
else
  error "Commit failed due to frontend code quality issues!"
fi

if npm --prefix assets test; then
  success "The frontend code tests passed."
else
  error "Commit failed due to frontend code test failures!"
fi

and my package.json contains:

{
  ...
  "scripts": {
    "build": "react-scripts build",
    "compile": "lingui compile --strict",
    "deploy": "webpack --mode production",
    "eject": "react-scripts eject",
    "extract": "lingui extract --clean ",
    "lint": "eslint --ext .js,.jsx,.ts,.tsx --max-warnings 0 .",
    "prettier": "prettier --check .",
    "prettier-write": "prettier --write .",
    "start": "react-scripts start",
    "test": "jest"
  },
  ...
}

So there are 5 commands related to node.js in the git hook script:

  • npm --prefix assets run extract
  • npm --prefix assets run compile
  • npm --prefix assets run prettier
  • npm --prefix assets run lint
  • npm --prefix assets test

Only the lingui commands exhibit the problem so there must be something done differently in lingui, I guess.

@andrii-bodnar andrii-bodnar closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants