Skip to content

BF: tolerate errors while parsing fetch lines - #1015

Merged
Byron merged 1 commit into
gitpython-developers:masterfrom
yarikoptic:bf-fetch-configmsg
May 31, 2020
Merged

BF: tolerate errors while parsing fetch lines#1015
Byron merged 1 commit into
gitpython-developers:masterfrom
yarikoptic:bf-fetch-configmsg

Conversation

@yarikoptic

Copy link
Copy Markdown
Contributor

At first I thought to provide special treatment to git config lines and
otherwise keep raising uncaught exception, but then decided that it might be
better to loose some progress information than to crash.
Also _get_push_info below is doing similarish catching of all exceptions
(although doesn't even log them).

With this change, log (if enabled and not suppressed) would show

[WARNING] Git informed while fetching: git config pull.rebase false # merge (the default strategy)

in the case of recently introduced change to the output in the following
git commit :

d18c950a69f3a24e1e3add3d9fc427641f53e12b is the first bad commit
commit d18c950a69f3a24e1e3add3d9fc427641f53e12b
Author: Alex Henrie <alexhenrie24@gmail.com>
Date: Mon Mar 9 21:54:20 2020 -0600
pull: warn if the user didn't say whether to rebase or to merge
Often novice Git users forget to say "pull --rebase" and end up with an
unnecessary merge from upstream. What they usually want is either "pull
--rebase" in the simpler cases, or "pull --ff-only" to update the copy
of main integration branches, and rebase their work separately. The
pull.rebase configuration variable exists to help them in the simpler
cases, but there is no mechanism to make these users aware of it.
Issue a warning message when no --[no-]rebase option from the command
line and no pull.rebase configuration variable is given. This will
inconvenience those who never want to "pull --rebase", who haven't had
to do anything special, but the cost of the inconvenience is paid only
once per user, which should be a reasonable cost to help a number of new
users.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/pull.c | 16 ++++++++++++++++
t/t5521-pull-options.sh | 22 +++++++++++-----------
t/t7601-merge-pull-config.sh | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+), 11 deletions(-)

Closes#1014

At first I thought to provide special treatment to git config lines and
otherwise keep raising uncaught exception, but then decided that it might be
better to loose some progress information than to crash.
Also _get_push_info below is doing similarish catching of all exceptions
(although doesn't even log them).
With this change, log (if enabled and not suppressed) would show
[WARNING] Git informed while fetching: git config pull.rebase false # merge (the default strategy)
in the case of recently introduced change to the output in the following
git commit :
d18c950a69f3a24e1e3add3d9fc427641f53e12b is the first bad commit
commit d18c950a69f3a24e1e3add3d9fc427641f53e12b
Author: Alex Henrie <alexhenrie24@gmail.com>
Date: Mon Mar 9 21:54:20 2020 -0600
pull: warn if the user didn't say whether to rebase or to merge
Often novice Git users forget to say "pull --rebase" and end up with an
unnecessary merge from upstream. What they usually want is either "pull
--rebase" in the simpler cases, or "pull --ff-only" to update the copy
of main integration branches, and rebase their work separately. The
pull.rebase configuration variable exists to help them in the simpler
cases, but there is no mechanism to make these users aware of it.
Issue a warning message when no --[no-]rebase option from the command
line and no pull.rebase configuration variable is given. This will
inconvenience those who never want to "pull --rebase", who haven't had
to do anything special, but the cost of the inconvenience is paid only
once per user, which should be a reasonable cost to help a number of new
users.
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/pull.c | 16 ++++++++++++++++
t/t5521-pull-options.sh | 22 +++++++++++-----------
t/t7601-merge-pull-config.sh | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+), 11 deletions(-)
Closesgitpython-developers#1014
@Byron
Byron merged commit a71ebbc into gitpython-developers:masterMay 31, 2020
@ByronByron added this to the v3.1.3 - Bugfixes milestone May 31, 2020
@Byron

Copy link
Copy Markdown
Member

Thanks a lot!
Since this is an issue many users could start seeing soon, I will cut a release right away.

@yarikoptic
yarikoptic deleted the bf-fetch-configmsg branch May 31, 2020 03:29
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

git v2.27.0-rc0~150^2 changed output on pull breaking GitPython parsing

2 participants

@yarikoptic@Byron