-
Notifications
You must be signed in to change notification settings - Fork 359
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
Fail when a git submodule fails to update instead of ignoring the error #6132
Conversation
cab43a1
to
ed71ca0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the code itself, lgtm!
Some remarks:
- The title of the PR doesn't fit the content, the second commit it more submodules handling than the failure itself. It needs to be updated by something more general, like "git submodules enhancements".
- Add a test to highlight the modifications, and follow them
100c9d6
to
c459fbd
Compare
I've split off the second commit in #6153 and added a test |
bffc94f
to
2a1e199
Compare
2a1e199
to
1d034d0
Compare
1d034d0
to
d442c01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
### ### This test may fail locally depending on your git version | ||
### ### File protocol is removed locally since git 2.38, | ||
### ### to fix a CVE: https://www.cve.org/CVERecord?id=CVE-2022-39253 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added that note
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That note isn't quite true. Only a couple of versions have file protocol disabled. More recent git versions work just fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah nevermind, what i'm saying is not true. My memory failed me.
Fixes #6131