Skip to content

Conversation

mknos
Copy link
Contributor

@mknos mknos commented Aug 7, 2024

  • patch failed with a strictness error when I specified a file that does not match the context of my diff
  • Patch::apply() calls back into Patch::Context::apply() on L714 with the wrong params
  • When setting a breakpoint in Patch::apply(), its params are not identical to Patch::Context::apply(), so passing @_ as parameter obviously fails
  • Reading the code, this would only be a problem for applying context diffs (no callback to self->apply() happened for the other diff types)
  • I confirmed that this error was not caused by the recent use-base.diff (now I suspect it goes back to 1999)
  • Removing the code with the incorrect self->apply() call causes patch to correctly report that applying Hunk1 failed, then the program exits with status 1 to indicate that >=1 lines could not be applied
%perl -d patch --dry-run expand ab.diff # stack trace from debugger shows Patch::apply() called with no list-ref params
...
Can't use string (" main(void)
") as an ARRAY ref while "strict refs" in use at patch line 844.
 at patch line 844.
	Patch::Context::apply(Patch::Context=HASH(0x22c8870), 5, 5, " main(void)\x{a}", " {\x{a}", " \x{9}puts(\"yo\");\x{a}", "-\x{9}printf(\"float=%lu double=%lu\\n\", sizeof(float), sizeof(doub"..., " \x{9}exit(1);\x{a}", ...) called at patch line 714
	Patch::apply(Patch::Context=HASH(0x22c8870), 5, 5, " main(void)\x{a}", " {\x{a}", " \x{9}puts(\"yo\");\x{a}", "-\x{9}printf(\"float=%lu double=%lu\\n\", sizeof(float), sizeof(doub"..., " \x{9}exit(1);\x{a}", ...) called at patch line 867
	Patch::Context::apply(Patch::Context=HASH(0x22c8870), 5, 5, ARRAY(0x21d3200), ARRAY(0x21cbbf8)) called at patch line 167

* patch failed with a strictness error when I specified a file
* Patch::apply() calls back into Patch::Context::apply() on L714 with the wrong params
* When setting a breakpoint in Patch::apply(), its params are not identical to Patch::Context::apply(), so passing @_ as parameter fails
* Reading the code, this would only be a problem for applying context diffs (no callback to self->apply() happened for the other diff types)
* I confirmed that this error was not caused by the recent use-base.diff (now I suspect it goes back to 1999)
* Removing the code with the incorrect self->apply() call causes patch to correctly report that applying Hunk briandfoy#1 failed, then the program exits with status 1 to indicate that >=1 lines could not be applied
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@github-actions github-actions bot added the Type: enhancement improve a feature that already exists label Aug 7, 2024
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@github-actions github-actions bot added Priority: low get to this whenever Program: patch The patch program labels Aug 7, 2024
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:11 — with GitHub Actions Inactive
@mknos mknos temporarily deployed to automated_testing August 7, 2024 02:12 — with GitHub Actions Inactive
@coveralls
Copy link

coveralls commented Aug 7, 2024

Pull Request Test Coverage Report for Build 10276936813

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.7%) to 73.375%

Totals Coverage Status
Change from base Build 10269273695: -0.7%
Covered Lines: 350
Relevant Lines: 477

💛 - Coveralls

@briandfoy
Copy link
Owner

changes: properly catch errors when applying a hunk fails.

@briandfoy briandfoy merged commit ea56ff0 into briandfoy:master Aug 7, 2024
22 of 23 checks passed
@briandfoy briandfoy self-assigned this Aug 7, 2024
@briandfoy briandfoy removed the Priority: low get to this whenever label Aug 7, 2024
@briandfoy briandfoy added the Status: accepted The fix is accepted label Aug 7, 2024
@briandfoy briandfoy added Status: released there is a new release with this fix and removed Status: accepted The fix is accepted labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Program: patch The patch program Status: released there is a new release with this fix Type: enhancement improve a feature that already exists
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants