Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Rapidquilt attempted to rollback a patch and that failed. #31

Closed
ptesarik opened this issue Dec 2, 2022 · 8 comments
Closed

Rapidquilt attempted to rollback a patch and that failed. #31

ptesarik opened this issue Dec 2, 2022 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@ptesarik
Copy link
Contributor

ptesarik commented Dec 2, 2022

The attached series triggers a bug.
Output with RUST_BACKTRACE=1 rapidquilt push -a --dry-run --threads=1 (just to make sure it's not a race condition, but it also fails multi-threaded):

Applying 1 patches single-threaded...
thread 'main' panicked at 'Rapidquilt attempted to rollback a patch and that failed. This is a bug. Failure report: FilePatchApplyReport { any_failed: true, hunk_reports: [Failed(NoMatchingLines), Skipped, Applied { line: 4850, rollback_line: 4850, offset: -26, line_count_diff: 31, fuzz: 0 }], direction: Revert, fuzz: 0, previous_permissions: None }', src/libpatch/patch/mod.rs:649:13
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: libpatch::patch::FilePatch<&[u8]>::rollback
   3: rapidquilt::apply::diagnostics::analyze_patch_failure
   4: rapidquilt::apply::sequential::apply_patches
   5: rapidquilt::cmd::run
   6: rapidquilt::main
@ptesarik ptesarik added the bug Something isn't working label Dec 2, 2022
@hramrach
Copy link
Contributor

hramrach commented Dec 2, 2022

Does it work with 0.6.5?

@ptesarik
Copy link
Contributor Author

ptesarik commented Dec 2, 2022

Does it work with 0.6.5?

No. Nor 0.6.4, or 0.6.3. Version 0.6.2 segfaults.

@ptesarik
Copy link
Contributor Author

ptesarik commented Dec 2, 2022

Just to make one thing clear, the series does NOT apply. This is the output of quilt push:

Applying patch s390-qeth-cache-link_info-for-ethtool
patching file qeth_core_main.c
Hunk #1 succeeded at 4849 (offset 105 lines).
Hunk #2 FAILED at 4753.
Hunk #3 FAILED at 4832.
2 out of 3 hunks FAILED -- rejects in file qeth_core_main.c
Patch s390-qeth-cache-link_info-for-ethtool does not apply (enforce with -f)

But it shouldn't trigger a bug.

@hramrach
Copy link
Contributor

Another instance:

Applying 66387 patches using 4 threads...
Saving modified files...
thread '' panicked at src/libpatch/patch/mod.rs:649:13:
Rapidquilt attempted to rollback a patch and that failed. This is a bug. Failure report: FilePatchApplyReport { any_failed: true, hunk_reports: [Applied { line: 241, rollback_line: 241, offset: -217, line_count_diff: 2, fuzz: 0 }, Failed(NoMatchingLines), Applied { line: 270, rollback_line: 272, offset: -221, line_count_diff: -3, fuzz: 0 }], direction: Revert, fuzz: 0, previous_permissions: Some(Permissions(FilePermissions { mode: 0o100444 (-r--r--r--) })) }
stack backtrace:
0: rust_begin_unwind
1: core::panicking::panic_fmt
2: libpatch::patch::FilePatch<&[u8]>::rollback
3: rayon::iter::plumbing::bridge_producer_consumer::helper
4: rayon::iter::plumbing::bridge_producer_consumer::helper
5: rayon::iter::plumbing::bridge_producer_consumer::helper
6: rapidquilt::apply::parallel::apply_patches
7: <rayon_core::job::StackJob<L,F,R> as rayon_core::job::Job>::execute
8: rayon_core::registry::WorkerThread::wait_until_cold
note: Some details are omitted, run with RUST_BACKTRACE=full for a verbose backtrace.

branch users/msuchanek/tmp/rapidquild-BUG in kernel-source

@ptesarik ptesarik self-assigned this Jan 7, 2025
@ptesarik
Copy link
Contributor Author

ptesarik commented Jan 7, 2025

Good news: I can still reproduce the same failure with rapidquilt. Looking into it now.

@ptesarik
Copy link
Contributor Author

ptesarik commented Jan 7, 2025

Status update: At least in one case, the bug is caused by overlapping hunks. Both hunks are applied, but since the second hunk changes the trailing context of the first hunk, the first one no longer matches in the reverse direction.

The apparent root cause is that overlapping hunks should fail to apply. That's a bug which must be fixed in any case. It may also be a good idea to roll back applied hunks in reverse order, which would mask the root cause at first, but it may allow to implement an option to allow overlapping hunks…

@ptesarik
Copy link
Contributor Author

ptesarik commented Jan 7, 2025

I have to correct myself. GNU patch accepts overlapping hunks, and rapidquilt can also parse them. As long as the overlap covers only the hunk context, all is fine. But if a hunk's context overlaps a previously patched line, the previous hunk is silently ignored! GNU patch will apply both (or all) hunks in that case.

At this point, it's better to allow overlapping hunks in rapidquilt.

@ptesarik
Copy link
Contributor Author

ptesarik commented Jan 8, 2025

Ugh, the behaviour of GNU patch is puzzling.

Let's have a file that goes like this:

aaa
bbb
ccc
ddd
eee
fff
ggg
hhh
iii
jjj
kkk
…

The following patch applies cleanly:

$ cat overlap.patch
@@ -1,7 +1,7 @@
 aaa
 bbb
 ccc
-ddd
+ddd modified
 eee
 fff
 ggg
@@ -2,7 +2,7 @@
 bbb
 ccc
 ddd
-eee
+eee modified
 fff
 ggg
 hhh
@@ -4,7 +4,7 @@
 ddd
 eee
 fff
-ggg
+ggg modified
 hhh
 iii
 jjj
$ patch --verbose -F0 -o file.out file.in < overlap.patch
Hmm...  Looks like a unified diff to me...
patching file file.out (read from file.in)
Using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 succeeded at 2.
Hunk #3 succeeded at 4.
done

Then I change the third hunk header to:

@@ -5,7 +5,7 @@

That still applies:

$ patch --verbose -F0 -o file.out file.in < overlap.patch
Hmm...  Looks like a unified diff to me...
patching file file.out (read from file.in)
Using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 succeeded at 2.
Hunk #3 succeeded at 4 (offset -1 lines).
done

Then I change the third hunk header to:

@@ -6,7 +6,7 @@

But then the hunk suddenly no longer applies:

$ patch --verbose -F0 -o file.out file.in < overlap.patch
Hmm...  Looks like a unified diff to me...
patching file file.out (read from file.in)
Using Plan A...
Hunk #1 succeeded at 1.
Hunk #2 succeeded at 2.
Hunk #3 FAILED at 6.
1 out of 3 hunks FAILED -- saving rejects to file file.out.rej
done

I'm afraid it's futile to provide bug-for-bug compatibility with GNU patch in this case. Instead, rapidquilt will also accept the third variant of the patch and apply the hunk with offset -2 lines.

ptesarik added a commit that referenced this issue Jan 8, 2025
This is related to #31, but not a complete solution yet, because I can still
reproduce the original failure with some patch series.

The last commit adds one of the reported cases to test data, and it passes
cargo test, so there is some improvement.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants