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

Log / error when git missing when running pip-tools #6617

Open
jeffwidman opened this issue Feb 7, 2023 · 4 comments
Open

Log / error when git missing when running pip-tools #6617

jeffwidman opened this issue Feb 7, 2023 · 4 comments
Labels
Batch How We Work: Feature. Outcome achieved within 1 iteration. Can live under an epic, or stand alone. good first issue L:python:pip-compile Python packages via pip-compile T: tech-debt ⚙️

Comments

@jeffwidman
Copy link
Member

jeffwidman commented Feb 7, 2023

See discussion here: #6609 (comment)

What happens if git isn't present?
If an error in creating a git repo results in pip-tools updates completely breaking, then it'd be nice to at least have an info log for easier debugging... don't have to capture the full subprocess, but instead we could log saying "couldn't create git repo, all pip-tools jobs will fail"...

And we probably should just outright bail here, no point in continuing the job if we know pip-tools will fail.

This was added in 4e72d4e which thankfully included a spec for the behavior.

Currently exists two places:

# Initialize a git repo to appease pip-tools
begin
run_command("git init") if setup_files.any?
rescue Dependabot::SharedHelpers::HelperSubprocessFailed
nil
end

and

# Initialize a git repo to appease pip-tools
command = SharedHelpers.escape_command("git init")
IO.popen(command, err: %i(child out)) if setup_files.any?

The second usage is surprising because it's in file_updater/pipfile_file_updater.rb, whereas pip-tools is actually pip-compile so I'd expect it to use file_updater/pip_compile_file_updater.rb instead. So possibly we don't need git there at all`, or possibly we do need it, but the code comment is wrong.

For the first one, we should also check if this is even still needed, as it may not be.

@jeffwidman
Copy link
Member Author

jeffwidman commented Feb 7, 2023

The root need for this might have been removed as part of:

I'll try it and see. 😁

@jeffwidman
Copy link
Member Author

jeffwidman commented Feb 8, 2023

Looks like removal breaks CI over in:

After looking deeper at the CI failure, it looks like the lack of git only causes breakage in a handful of situations. So we shouldn't raise, just need to add an info log.

This isn't a super high risk issue and it's very approachable for someone new to the project, so I'm going to tag it as a "good first issue" and leave it for a potential contributor. You can use this PR as an example of how to add a new log line:

@jeffwidman
Copy link
Member Author

@abdulapopoola abdulapopoola added the Batch How We Work: Feature. Outcome achieved within 1 iteration. Can live under an epic, or stand alone. label Mar 14, 2024
@GarryHurleyJr
Copy link
Collaborator

Please leave this one for new joining person as it should be simple

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Batch How We Work: Feature. Outcome achieved within 1 iteration. Can live under an epic, or stand alone. good first issue L:python:pip-compile Python packages via pip-compile T: tech-debt ⚙️
Projects
Status: Ready
Development

No branches or pull requests

4 participants
@jeffwidman @abdulapopoola @GarryHurleyJr and others