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.

Sign up for GitHub

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

Jump to bottom

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

Closed
yarikoptic opened this issue May 30, 2020 · 0 comments · Fixed by #1015
Closed

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

yarikoptic opened this issue May 30, 2020 · 0 comments · Fixed by #1015
Labels
critical
Milestone
v3.1.3 - Bugfixes

Comments

@yarikoptic
Copy link
Contributor

yarikoptic commented May 30, 2020 •
edited
Loading

our original issue in datalad: datalad/datalad#4592

found issue in debian initially against fdroidserver then reassigned against GitPython (python3-git pkg)

  File "/build/datalad-0.12.7/.pybuild/cpython3_3.8_datalad/build/datalad/support/gitrepo.py", line 2209, in _call_gitpy_with_progress
    ret = callable(**git_kwargs)
  File "/usr/lib/python3/dist-packages/git/remote.py", line 814, in pull
    res = self._get_fetch_info_from_stderr(proc, progress)
  File "/usr/lib/python3/dist-packages/git/remote.py", line 710, in _get_fetch_info_from_stderr
    output.extend(FetchInfo._from_line(self.repo, err_line, fetch_line)
  File "/usr/lib/python3/dist-packages/git/remote.py", line 710, in <genexpr>
    output.extend(FetchInfo._from_line(self.repo, err_line, fetch_line)
  File "/usr/lib/python3/dist-packages/git/remote.py", line 294, in _from_line
    raise ValueError("Failed to parse line: %r" % line)
ValueError: Failed to parse line: '  git config pull.rebase false  # merge (the default strategy)'

bisected to

d18c950a69f3a24e1e3add3d9fc427641f53e12b is the first bad commit
commit d18c950a69f3a24e1e3add3d9fc427641f53e12b
Author: Alex Henrie [email protected]
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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

builtin/pull.c | 16 ++++++++++++++++
t/t5521-pull-options.sh | 22 +++++++++++-----------
t/t7601-merge-pull-config.sh | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 65 insertions(+), 11 deletions(-)

and manifests in appearing the line
@yarikoptic yarikoptic mentioned this issue May 30, 2020
Closed
yarikoptic added a commit to yarikoptic/GitPython that referenced this issue May 30, 2020
@yarikoptic
BF: tollerate errors while parsing fetch lines
85ea5d7 
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 <[email protected]>
	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 <[email protected]>
		Signed-off-by: Junio C Hamano <[email protected]>

	 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 gitpython-developers#1014
@yarikoptic yarikoptic mentioned this issue May 30, 2020
Merged
Byron pushed a commit that referenced this issue May 31, 2020
@yarikoptic @Byron
BF: tollerate errors while parsing fetch lines
a71ebbc 
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 <[email protected]>
	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 <[email protected]>
		Signed-off-by: Junio C Hamano <[email protected]>

	 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
@Byron Byron added this to the v3.1.3 - Bugfixes milestone May 31, 2020
@msiemens msiemens mentioned this issue Aug 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
critical
Milestone
v3.1.3 - Bugfixes
Development

Successfully merging a pull request may close this issue.

BF: tolerate errors while parsing fetch lines yarikoptic/GitPython
2 participants
@yarikoptic @Byron