Bug 1469098 - git am doesn't apply for base64 encoded mail
git am doesn't apply for base64 encoded mail
Status: NEW
Product: Fedora
Classification: Fedora
Component: git (Show other bugs)
27
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: pstodulk
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2017-07-10 08:08 EDT by Jaroslav Škarvada
Modified: 2017-11-12 11:58 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Reproducer (10.83 KB, message/rfc822)
2017-07-10 08:08 EDT, Jaroslav Škarvada
no flags Details

  None (edit)
Description Jaroslav Škarvada 2017-07-10 08:08:12 EDT
Created attachment 1295789 [details]
Reproducer

Description of problem:
It seems it strips CR in mailsplit during preprocessing but it doesn't strip them later after base64 decoding.

Version-Release number of selected component (if applicable):
git-2.13.2-1

How reproducible:
Always

Steps to Reproduce:
1. git clone https://github.com/redhat-performance/tuned.git
2. git checkout -b test 55c7128d1c3cb8e85b4b1c73ebac9da6235bd487
3. git am ./reproducer

Actual results:
Applying: Python traces are seen when tuned-adm verifiy -i is executed for cpu-partitioning profile.
.git/rebase-apply/patch:14: trailing whitespace.
	try:
.git/rebase-apply/patch:15: trailing whitespace.
           interruptdirs.remove("2")
.git/rebase-apply/patch:16: trailing whitespace.
        except ValueError:
.git/rebase-apply/patch:17: trailing whitespace.
           pass
.git/rebase-apply/patch:21: trailing whitespace.
	try:
error: patch failed: libexec/defirqaffinity.py:61
error: libexec/defirqaffinity.py: patch does not apply
Patch failed at 0001 Python traces are seen when tuned-adm verifiy -i is executed for cpu-partitioning profile.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Expected results:
It applies

Additional info:
If I decode the base64 encoded part with the 'base64 --decode' add the e-mail original header and remove the 'Content-Transfer-Encoding: base64' line, the 'git am' applies OK.
Comment 1 Jan Kurik 2017-08-15 03:20:15 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 27 development cycle.
Changing version to '27'.
Comment 2 Todd Zullinger 2017-11-12 11:58:50 EST
The patch applies if you pass --ignore-whitespace to git am:

$ git am --ignore-whitespace /tmp/reproducer.patch
Applying: Python traces are seen when tuned-adm verifiy -i is executed for cpu-partitioning profile.

Testing this, I first ran git am with --reject and with GIT_TRACE set, to see where it went wrong:

$ GIT_TRACE=1 git am --reject /tmp/reproducer.patch
11:36:13.090859 git.c:344               trace: built-in: git 'am' '--reject' '/tmp/reproducer.patch'
11:36:13.091754 run-command.c:626       trace: run_command: 'mailsplit' '-d4' '-o.git/rebase-apply' '-b' '--' '/tmp/reproducer.patch'
11:36:13.093487 git.c:344               trace: built-in: git 'mailsplit' '-d4' '-o.git/rebase-apply' '-b' '--' '/tmp/reproducer.patch'
Applying: Python traces are seen when tuned-adm verifiy -i is executed for cpu-partitioning profile.
Checking patch libexec/defirqaffinity.py...
error: while searching for:
	# now verify each /proc/irq/$num/smp_affinity?
	interruptdirs = [ f for f in os.listdir(irqpath) if os.path.isdir(os.path.join(irqpath,f)) ]?
	# IRQ 2 - cascaded signals from IRQs 8-15 (any devices configured to use IRQ 2 will actually be using IRQ 9)?
	interruptdirs.remove("2")?
	# IRQ 0 - system timer (cannot be changed)?
	interruptdirs.remove("0")?
?
	for i in interruptdirs:?
		inplacemask = 0?
		fname = irqpath + i + "/smp_affinity"?

error: patch failed: libexec/defirqaffinity.py:61
Applying patch libexec/defirqaffinity.py with 1 reject...
Rejected hunk #1.
Patch failed at 0001 Python traces are seen when tuned-adm verifiy -i is executed for cpu-partitioning profile.
The copy of the patch that failed is found in: .git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

The "?" made me suspicious that it was a whitespace issue.  Checking the content of the extracted patch shows it's got CRLF line ends:

$ file .git/rebase-apply/patch 
.git/rebase-apply/patch: unified diff output, ASCII text, with CRLF, LF line terminators

This is present in the base64-decoded output as well:

$ sed '1,/^$/d' /tmp/reproducer.patch | base64 -d - | file -
/dev/stdin: unified diff output, ASCII text, with CRLF, LF line terminators

It does seem that git is better able to apply the patch after it's decoded and it might be nice if it did that when decoding base64 content as well.  But IMO, the patch is fundamentally whitespace damaged.  (Whether that damage came from the sender or was mangled by the mailing-list or other software on route, I can't say.)

Additionally, as well as the bogus CRLF line ends, the patch uses spaces for indentation rather than tabs as the rest of the code does.  With python, this is at best sloppy and at worst a bug which will prevent proper execution.  I tend to include -tt in the shebang of my python scripts make such random mixing of tabs and spaces an error:

$ python --help
...
-t     : issue warnings about inconsistent tab usage (-tt: issue errors)

(I am more sensitive to whitespace than many people though.  I've seen it cause plenty of bugs in shell scripts, python code, and elsewhere.)

Note You need to log in before you can comment on or make changes to this bug.