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

[Build] fail silently on register git hook failure #852

Merged
merged 1 commit into from
Oct 11, 2021

Conversation

kavilla
Copy link
Member

@kavilla kavilla commented Oct 7, 2021

Description

node scripts/register_git_hook fails depending on the installed
git version because it ends up calling git rev-parse --git-common-dir.
Depending on the git version, --git-common-dir returns the literal
string --git-common-dir when it is suppose to return $GIT_COMMON_DIR
if set and, if not, $GIT_DIR. Then it proceeds to try to install from a
path that doesn't exist because it will use --git-common-dir in the
path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
#851

Signed-off-by: Kawika Avilla [email protected]

Issues Resolved

#850

Check List

  • New functionality includes testing. (nothing is exported in that file)
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

Sorry, something went wrong.

Copy link
Contributor

@boktorbb boktorbb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the Node CI is failing. Not sure if it’s intended because of the change or not

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed b943712

@kavilla
Copy link
Member Author

kavilla commented Oct 7, 2021

Looks like the Node CI is failing. Not sure if it’s intended because of the change or not

Yeah looking into it's saying typescript version error which is strange.

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 41ac938

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed a34db37

`node scripts/register_git_hook` fails depending on the installed
git version because it ends up calling `git rev-parse --git-common-dir`.
Depending on the git version, `--git-common-dir` returns the literal
string `--git-common-dir` when it is suppose to return $GIT_COMMON_DIR
if set and $GIT_DIR if the previous is not set. Then it proceeds to try
to install from a path that doesn't exist because it will use
`--git-common-dir` in the path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
opensearch-project#851

Issue resolved:
opensearch-project#850

Signed-off-by: Kawika Avilla <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 42211dd

@dblock
Copy link
Member

dblock commented Oct 8, 2021

Hacky but works.

@kavilla kavilla merged commit 38ace12 into opensearch-project:main Oct 11, 2021
@kavilla kavilla deleted the avillk/gitcheck branch October 11, 2021 06:00
kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Oct 11, 2021
`node scripts/register_git_hook` fails depending on the installed
git version because it ends up calling `git rev-parse --git-common-dir`.
Depending on the git version, `--git-common-dir` returns the literal
string `--git-common-dir` when it is suppose to return $GIT_COMMON_DIR
if set and $GIT_DIR if the previous is not set. Then it proceeds to try
to install from a path that doesn't exist because it will use
`--git-common-dir` in the path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
opensearch-project#851

Issue resolved:
opensearch-project#850

Backport PR:
opensearch-project#852

Signed-off-by: Kawika Avilla <[email protected]>
kavilla added a commit to kavilla/OpenSearch-Dashboards-1 that referenced this pull request Oct 11, 2021
`node scripts/register_git_hook` fails depending on the installed
git version because it ends up calling `git rev-parse --git-common-dir`.
Depending on the git version, `--git-common-dir` returns the literal
string `--git-common-dir` when it is suppose to return $GIT_COMMON_DIR
if set and $GIT_DIR if the previous is not set. Then it proceeds to try
to install from a path that doesn't exist because it will use
`--git-common-dir` in the path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
opensearch-project#851

Issue resolved:
opensearch-project#850

Backport PR:
opensearch-project#852

Signed-off-by: Kawika Avilla <[email protected]>
kavilla added a commit that referenced this pull request Oct 13, 2021
`node scripts/register_git_hook` fails depending on the installed
git version because it ends up calling `git rev-parse --git-common-dir`.
Depending on the git version, `--git-common-dir` returns the literal
string `--git-common-dir` when it is suppose to return $GIT_COMMON_DIR
if set and $GIT_DIR if the previous is not set. Then it proceeds to try
to install from a path that doesn't exist because it will use
`--git-common-dir` in the path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
#851

Issue resolved:
#850

Backport PR:
#852

Signed-off-by: Kawika Avilla <[email protected]>
kavilla added a commit that referenced this pull request Oct 13, 2021
`node scripts/register_git_hook` fails depending on the installed
git version because it ends up calling `git rev-parse --git-common-dir`.
Depending on the git version, `--git-common-dir` returns the literal
string `--git-common-dir` when it is suppose to return $GIT_COMMON_DIR
if set and $GIT_DIR if the previous is not set. Then it proceeds to try
to install from a path that doesn't exist because it will use
`--git-common-dir` in the path.

The goal for this fix is to bypass hard failures in 1.x since I do
not believe it is appropriate to fail building something due to git.
But I do not want to just remove adding the pre-commit git hook
if people are already used to it being installed while bootstrapping.

Here, I propose the removal of git hooks on bootstrap in a 2+ version
of Dashboards:
#851

Issue resolved:
#850

Backport PR:
#852

Signed-off-by: Kawika Avilla <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: OpenSearch Dashboards register git hook failure due to git version
5 participants