Fix fatal config set error

[sepehr-rs]

Fixes #13279

I tried fixing this by providing the user with a helpful error message instead of crashing, and made a new parser to allow writing to the PIP_CONFIG_FILE environment variable temporarily.

One important caveat is that changes made to the config file via PIP_CONFIG_FILE may not be reflected in other commands like pip config get due to how pip loads config scopes.

Would appreciate any feedback on this :).

[sepehr-rs]

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

[notatallshaw]

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

Do you know if there is anything particular about your git workflow? It does seem to be happening with your PRs more than anyone else's (though I have seen it happen at least once to mine).

You could perhaps report this to pre-commit, I would normally assist in reporting a bug but my experience with that project means I am not inclined to help it out.

In the mean time I'm thinking we should move off pre-commit.ci, and instead just add it as a github workflow, I don't see any advantage to having it run in a separate infrastructure. I'll make an issue and a PR when I have a moment.

[sepehr-rs]

I think pre-commit is showing weird behavior here again...
I've changed the problematic files but it is still failing with the old files.

Do you know if there is anything particular about your git workflow? It does seem to be happening with your PRs more than anyone else's (though I have seen it happen at least once to mine).

You could perhaps report this to pre-commit, I would normally assist in reporting a bug but my experience with that project means I am not inclined to help it out.

In the mean time I'm thinking we should move off pre-commit.ci, and instead just add it as a github workflow, I don't see any advantage to having it run in a separate infrastructure. I'll make an issue and a PR when I have a moment.

No, I don’t think the issue is on my end. It might be that committing too quickly causes these problems. And sure, I’ll report this as a potential bug to pre-commit. Thanks for your feedback!

[notatallshaw]

Oh great, Windows CI is failing today 😞 , I'll take a look in a min.

[notatallshaw]

Seems like it was a transient error, I'll see if I can add some retry logic on the installing installing SVN step in the future.

[notatallshaw]

I don't think this PR fully addresses all the review comments yet, I will try and take a look in a few weeks.

[sepehr-rs]

Gentle ping. I believe this is ready for review. This PR also addresses #12706, #11880, and #12481. I'd appreciate any feedback when someone has time.

[notatallshaw]

Thanks for the ping, I'm going to be away on vacation for a week, but I hope some time in June to sit down and do make a big dent in the backlog of things I've promised to review.

[notatallshaw]

The except IndexError in get_file_to_edit is now dead, the fallback returns or raises ConfigurationError.

Would be good to have some tests for unset + PIP_CONFIG_FILE, or some unit or functional tests in general.

[sepehr-rs]

Thanks so much for the thorough reviews, @notatallshaw!

I've addressed the remaining points, got unset working, made the code raise an exception when PIP_CONFIG_FILE is set to devnull, and added tests for both.

I'd appreciate another look when you have a chance!

[sepehr-rs]

Hey @ichard26, would you be open to considering adding this to the upcoming release milestone? I believe the latest review comments have been addressed and the requested tests have been added. No worries if it's too late in the release cycle or you'd prefer to leave it for a later release.

[ichard26]

TBH, pip's configuration code is one of my least favourite parts of the codebase to make changes. It's a mess to follow¹ while being an easy source of backwards compatibility potholes. I'd need to invest a significant amount of time to understand what's the underlying issue here and whether this change is the right solution.

So, I consider this PR out of scope for the upcoming 26.2 release. I'll tack to the 26.3 milestone for the time being.

Footnotes

1. Not critiquing the code itself or those who wrote any of it, but it's impenetrably hard to read ↩