savannah: Can't parse unified diff header, occurence of /^---/

[hrazdii]

Original: ​http://savannah.gnu.org/bugs/?24906

Submitted by:	Igor Hrazdil <hrazdii>	Submitted on:	Sun 23 Nov 2008 08:59:26 AM UTC
Category:	None	Severity:	3 - Normal
Status:	None	Privacy:	Public
Assigned to:	None	Open/Closed:	Open
Release:	None	Operating System:	None

Discussion:

Sun 23 Nov 2008 08:59:26 AM UTC, comment #5:

This item has been reassigned from the project Gnash - The GNU Flash
 player bugs tracker to your tracker.

The original report is still available at bugs #24885

Following are the information included in the original report:

[field #0] Item ID: 24885
[field #1] Group ID: 8191
[field #2] Open/Closed: Open
[field #3] Severity: 3 - Normal
[field #4] Privacy: Public
[field #8] : Unknown bugs Field Display Type
[field #9] Category: utilities
[field #10] Submitted by: hrazdii
[field #11] Assigned to: None
[field #12] Submitted on: Do 20 Nov 2008 06:17:52 CET
[field #13] Summary: Can't parse unified diff header, occurence of /^---/
[field #14] Original Submission: Problem: In my patch file is "---" 
on the beginning of line.

For better explanation i was changed my patchfs script:
# diff patchfs_ patchfs
142c142
< error "Can't parse unified diff header"
---
> error "Can't parse unified diff header >>$$buf<<"

The script output is:
$ /usr/share/mc/extfs/patchfs list adonis.patch.bz2
-rw-r--r-- 1 igor igor 566 02-09-2008 04:00 adonis/bin/adv_sugg_maintain.sh.diff
-rw-r--r-- 1 igor igor 1936 09-05-2008 23:52 adonis/bin/archiv.sh.diff
Can't parse unified diff header >>--- where ar.last_update >= "2008-09-25"
-group by 1,2
-order by 1,2;
<<

In one of original data file is line with "--" (a sql comment 
without blank space on the bebinning):
-- where ar.last_update >= "2008-09-25"

[field #16] Item Group: None
[field #17] Status: None
[field #18] Component Version: None
[field #19] Operating System: None
[field #20] Reproducibility: None
[field #21] Size (loc): None
[field #22] Fixed Release: None
[field #23] Planned Release: None
[field #24] Effort: 0.00
[field #28] Priority: 5 - Normal
[field #31] Percent Complete: 0%
[field #33] Release: None
[field #58] Custom Select Box #1: None
[field #59] Custom Select Box #2: None
[field #60] Custom Select Box #3: None
[field #61] Custom Select Box #4: None
[field #62] Custom Select Box #5: None
[field #63] Custom Select Box #6: None
[field #64] Custom Select Box #7: None
[field #65] Custom Select Box #8: None
[field #66] Custom Select Box #9: None
[field #67] Custom Select Box #10: None

	Benjamin Wolsey <bwy>
Sun 23 Nov 2008 08:59:23 AM UTC, comment #4:

Looks like one of your bugs has got lost, mc. Greetings from the 
Gnash team.
	Benjamin Wolsey <bwy>
Thu 20 Nov 2008 07:12:46 AM UTC, comment #3:

What is this about, please?
	Benjamin Wolsey <bwy>
Thu 20 Nov 2008 06:47:36 AM UTC, comment #2:

a clumsy single-purpose patch is:

--- patchfs_ 2008-11-20 06:07:51.000000000 +0100
+++ patchfs 2008-11-20 07:42:18.000000000 +0100
@@ -10,6 +10,7 @@
use strict;
use POSIX;
use File::Temp 'tempfile';
+use IO::Handle;

# standard binaries
my $bzip = 'bzip2';
@@ -149,6 +150,15 @@
}
}

+sub _b
+{
+ my ($buf, $c)=@_;
+ return if $$buf !~ /^--- /;
+ $c = IO::Handle::getc(*I);
+ IO::Handle::ungetc(*I, ord($c));
+ return 1 if ord($c) == 43; # ord("+")
+}
+
# list files affected by patch
sub list
{
@@ -168,7 +178,7 @@

# recognize diff type
if (!$unified && !$context) {
- $unified=1 if (/^--- /);
+ $unified=1 if b(\$);
$context=1 if (/^\\\* /);
if (!$unified && !$context) {
$len+=length;
@@ -176,7 +186,7 @@
}
}

- if (($unified && /^--- /) || ($context && /^\\\* [^\]$/)) {
+ if (($unified && b(\$)) || ($context && /^\\\* [^\]$/)) {
# start of new file
if ($state==1) {
printf "-rw-r--r-- 1 %s %s %d %s %s%s\n", $uid, $gid, $len, datetime($time), $prefix, $f
	Igor Hrazdil <hrazdii>
Thu 20 Nov 2008 05:23:21 AM UTC, comment #1:

I forget, the patchfs script version is 1.17:
http://cvs.savannah.gnu.org/viewvc/mc/vfs/extfs/patchfs.in?revision=1.17&root=mc&view=markup
	Igor Hrazdil <hrazdii>
Sun 23 Nov 2008 08:59:26 AM UTC, original submission:

Problem: In my patch file is "---" on the beginning of line.

For better explanation i was changed my patchfs script:
# diff patchfs_ patchfs
142c142
< error "Can't parse unified diff header"
---

> error "Can't parse unified diff header >>$$buf<<"


The script output is:
$ /usr/share/mc/extfs/patchfs list adonis.patch.bz2
-rw-r--r-- 1 igor igor 566 02-09-2008 04:00 adonis/bin/adv_sugg_maintain.sh.diff
-rw-r--r-- 1 igor igor 1936 09-05-2008 23:52 adonis/bin/archiv.sh.diff
Can't parse unified diff header >>--- where ar.last_update >= "2008-09-25"
-group by 1,2
-order by 1,2;
<<

In one of original data file is line with "--" (a sql comment 
without blank space on the bebinning):
-- where ar.last_update >= "2008-09-25" 

[kdave]

I'm taking this one. I've managed to create a valid diff which is misparsed by extfs/patchfs. Currently the unified diff parsing for 'list' command is done.

[kdave]

testfile: valid diff which is misparsed with current patchfs

[kdave]

rewritten parser of unified diff files, copyin is disabled

[kdave]

So, here's rewritten parser of unified diffs with a testfile. You can test it by running

patchfs list patchfs-testcase.diff

It does more fine-grained parsing of hunks and is about 2 times slower, still fast enough when tested on 20MB diff.

The script does not allow the 'copyin' command. Why? What should copyin to diff file mean? There are a few options:

• copyin of a non-diff file into archive -- just appends the data, but never displays it back (parsed as a diff comment and thrown away)
• copyin of a diff file, which contains hunks of files already in the archive -- now comes the tricky part:

1. delete all data from previous file and replace them with the copyin contents
2. just append the new hunks -- this may produce invalid diff, hunks in reversed order or changing same lines from more hunks
3. merge hunks of the files and produce a valid diff -- do we really want to rewrite patchutils?

Should user choose what to do for copyin? Does anybody rely on this feature? (Rely to that extent that it cannot be done in other way?)

Given that, I'd rather disable copyin.

[angel_il]

Replying to kdave:
is still actual for current

[kdave]

Replying to angel_il:

is still actual for current

Sorry, do you ask if this is still actual for current git? Yes. AFAICS, nobody committed it to master or any branch.

[angel_il]

Sorry very-very mach. :)
is still actual for current master?

[angel_il]

Replying to kdave:

Replying to angel_il:Yes. AFAICS, nobody committed it to master or any branch.

ok, i'll create branch 90_

[angel_il]

Branch: 90_patchfs
changeset: a821f1c9897e42439272a2fdf06ceced7e7ad7bf

[slavazanko]

good fix.

[slavazanko]

ps, no vote

Branch will remove xz-stuff. Not good.

[angel_il]

branch 90_patchfs
new changeset :b29e81556f67087955a9eceec45347d002c781f1

ps:sorry now i can't vote becouse i change this source.

[slavazanko]

Now it's really look good :)

[andrew_b]

Agree. My vote here.

[angel_il]

Fixed:
changeset: b29e81556f67087955a9eceec45347d002c781f1