Check git at startup

[rarkins]

What would you like Renovate to be able to do?

Check git version and error if it's not correct.

Did you already have any implementation ideas?

We may need to make it a log error only and not exit immediately if there's the chance of false positives.

[arielferdman]

I would like to do this issue

[RahulGautamSingh]

I want to work on this issue. It is my first issue so, I want to get it right.

I have added an esm js file in the tools folder of my local repo, which can get the Git version of the user. It works fine on Windows when I run it manually.

Should I write the test cases to check the script? Or can I submit a PR with this script as is?

[rarkins]

There's two ways we ideally want to use such validation:

1. During tests (both local as well as CI) to alert us if the git is not high enough and could cause failing tests, and
2. In production, at startup, to warn if the underlying git is too old and abort

If you have written a tools script then it could satisfy (1) if it's added to yarn test. Maybe it could even be a good thing if it is run as part of yarn lint and aborts unit testing if it fails. However it is likely not good for (2).

I think if you submit (1) and incorporate it as part of yarn lint then you do not need to add tests, because it is the test.

[RahulGautamSingh]

I wrote the script and added it to lint but there is a catch ,when I try running the code I get this error:
2021/09/10 16:16:00 ./tools/check_compatibility.js failed for rules: kebabcase
I could not get it to work correctly even after changing the variable names many times so, finally I tried ignoring tests on the script file by adding it to ignore section in .ls-lint.yml and then the workflow tests went fine.
I needed to know whether I should change my script(I would need some advice on this one) or I can ignore the kebabcase tests on the script.

[rarkins]

It's the file name underscore. Use "check-git-version.js"

[RahulGautamSingh]

Oh, my bad I didn't change the filename. I will try that and get back to you.

[RahulGautamSingh]

The workflow tests have passed now. Can you take a look?
https://github.com/RahulGautamSingh/renovate/tree/feat/check-git-version

[rarkins]

Raise a draft PR if you'd like a review

[HonkingGoose]

Are PRs #11687 and #11767 enough to close this issue? Or do we need to do more work before we can consider this issue done?

[viceice]

We like to test it at runtime startup too, so no. 😉

[HonkingGoose]

We like to test it at runtime startup too, so no. 😉

Oh okay, figured it was worth checking! 😄

[RahulGautamSingh]

We like to test it at runtime startup too, so no. 😉

Oh okay, figured it was worth checking! 😄

Has this issue been tested at runtime startup?

[rarkins]

This script is not the way it should be done, it should be as part of the application.

[olegkrivtsov]

I'd like to help with this one.

[viceice]

checkout #9979 for a starter

[olegkrivtsov]

As far as I can see #9979 implements exactly what is required. I see that PR is Merged, but I don't see that changes in current main:

renovate/lib/workers/global/initialize.ts

Lines 37 to 47 in d7713a3

	export async function globalInitialize(
	config_: RenovateConfig
	): Promise<RenovateConfig> {
	let config = config_;
	config = await initPlatform(config);
	config = await setDirectories(config);
	packageCache.init(config);
	limitCommitsPerRun(config);
	setEmojiConfig(config);
	return config;
	}

So my plan for fixing this issue would be: take PR #9979 as a base and basicly do the same. @viceice does that make sense?

[rarkins]

See #10106

IIRC it was failing on Windows CI

[HonkingGoose]

Direct quote from yourself: ¹

Fails on windows CI :) .. not sure yet if it's because of the version or something else. Will add windows tests to the replacement PR

Footnotes

1. https://github.com/renovatebot/renovate/pull/10106#issuecomment-846368995 ↩

[viceice]

I think we now have a label to force a full ci run to catch the error. 🤗

[olegkrivtsov]

I tried to google "Windows CI", and no luck ( Could you please advice where I can find some info/instructions on how to run Renovate in Windows CI?

[rarkins]

This repo. We don't test in windows in PRs by default but we do once it hits main branch

[olegkrivtsov]

My plan for fixing this issue would be to take PR #9979 as a base and basicly do the same, but also run it in Windows and check why it doesn't work in Windows. Is it a good plan?

[rarkins]

As @viceice pointed out, I think we now have a way to run the full tests (including windows) on-demand in PRs. So I suggest re-raising the same code, getting the CI tests to run, and then investigating from there.

[olegkrivtsov]

Could you please advice how to enable the label for full ci run? Should I check the docs of GitHub Actions? https://docs.github.com/en/actions

[viceice]

@olegkrivtsov Please open a draft pr, then we add ci:fulltest label, which causes full ci run on next push (including windows and macos)

[renovate-release]

🎉 This issue has been resolved in version 29.35.0 🎉

The release is available on:

• GitHub release
• 29.35.0

Your semantic-release bot 📦🚀