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.

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

Parse and apply all the unified patch files from opam-repository #22

Merged
merged 21 commits into from
Mar 6, 2025

Conversation

kit-ty-kate
Copy link
Collaborator

In its current state, this PR fixes all parsing error. However some patch files are simply ignored as they use different formats (e.g. contextual diff, which can unfortunately be found in opam-repository)

This will be a draft until i manage to ensure no difference between GNU patch and this version of patch for every packages in opam-repository (this will also require #18)

@hannesm
Copy link
Owner

hannesm commented Oct 8, 2024

n8ce. would be great to add these patch files as regression tests to the testsuite here (also the contextual diffs).

@kit-ty-kate kit-ty-kate changed the title Parse all the patch files from opam-repository Parse and apply all the patch files from opam-repository Oct 11, 2024
@kit-ty-kate
Copy link
Collaborator Author

Week update: using ocaml/opam#5892 and this PR, only 9 out of the 915 packages with patches differ from GNU patch.
The remaining diff is:

Files opam-2.3.0-alpha1/dbm.1.0/Makefile and opam+ocaml-patch/dbm.1.0/Makefile differ
Only in opam+ocaml-patch/frama-c.22.0/tests/fc_script/oracle: flamegraph.err
Only in opam+ocaml-patch/frama-c.22.0/tests/fc_script/oracle: flamegraph.res
Files opam-2.3.0-alpha1/libsvm.0.8.3/examples/svm_cli.ml and opam+ocaml-patch/libsvm.0.8.3/examples/svm_cli.ml differ
Files opam-2.3.0-alpha1/libsvm.0.8.3/lib/libsvm.ml and opam+ocaml-patch/libsvm.0.8.3/lib/libsvm.ml differ
Only in opam-2.3.0-alpha1/mikmatch.1.0.7: Makefile.orig
Only in opam-2.3.0-alpha1/mikmatch.1.0.7: Makefile.rej
Files opam-2.3.0-alpha1/nocrypto.0.5.4-1/_tags and opam+ocaml-patch/nocrypto.0.5.4-1/_tags differ
Files opam-2.3.0-alpha1/nocrypto.0.5.4-1/pkg/META.in and opam+ocaml-patch/nocrypto.0.5.4-1/pkg/META.in differ
Files opam-2.3.0-alpha1/nocrypto.0.5.4-2/_tags and opam+ocaml-patch/nocrypto.0.5.4-2/_tags differ
Files opam-2.3.0-alpha1/nocrypto.0.5.4-2/pkg/META.in and opam+ocaml-patch/nocrypto.0.5.4-2/pkg/META.in differ
Files opam-2.3.0-alpha1/ocp-indent.0.6.0/src/ocp-indent.ocp and opam+ocaml-patch/ocp-indent.0.6.0/src/ocp-indent.ocp differ
Files opam-2.3.0-alpha1/ocp-indent.0.6.2/src/ocp-indent.ocp and opam+ocaml-patch/ocp-indent.0.6.2/src/ocp-indent.ocp differ
Files opam-2.3.0-alpha1/ocp-indent.0.9.0/src/ocp-indent.ocp and opam+ocaml-patch/ocp-indent.0.9.0/src/ocp-indent.ocp differ
Files opam-2.3.0-alpha1/ocp-index.1.1.4/opam and opam+ocaml-patch/ocp-index.1.1.4/opam differ
Files opam-2.3.0-alpha1/ocp-index.1.1.4/src/grepMain.ml and opam+ocaml-patch/ocp-index.1.1.4/src/grepMain.ml differ
Files opam-2.3.0-alpha1/ocp-index.1.1.4/src/indexBuild.ml and opam+ocaml-patch/ocp-index.1.1.4/src/indexBuild.ml differ
Files opam-2.3.0-alpha1/ocp-index.1.1.4/src/indexOut.ml and opam+ocaml-patch/ocp-index.1.1.4/src/indexOut.ml differ
Files opam-2.3.0-alpha1/ocp-index.1.1.4/src/indexPredefined.ml and opam+ocaml-patch/ocp-index.1.1.4/src/indexPredefined.ml differ

I hope to finish it next week. This will require #19, allowing spaces instead of tabs between the date and the filename in some cases (that's going to be annoying but i have ideas on how to fix it) and allowing contextual diffs.
This PR fixes #18 already and it seems to work quite well. I'll improve the API to be able to show when a non-exact diff was applied later.

If anyone wants to help this work: The best way to help would be to add patches and expected results to the testsuite. I've done my best to give an example of package failing before succeeding after for most commits.
I'll also drop my local testsuite (opam-repository) next week but that one will have to live as a script to be run manually.

@kit-ty-kate kit-ty-kate force-pushed the opam-repository-fixes branch from 7db379a to 4c6b846 Compare January 21, 2025 00:22
@kit-ty-kate
Copy link
Collaborator Author

kit-ty-kate commented Jan 21, 2025

I'm done i think, there are no differences between the results of the patches applications from opam-repository with GNU patch and with ocaml-patch, except when there are context diffs involved which will now fail.
I give up trying to support context diff and raise an error instead, the rest of the opam maintainers agreed to stop supporting the context diff format which isn't used anywhere else other than very old packages.

Next i'll do a cleanup, rebase and this should be good to merge and released as an alpha version for further testing during the dev and alpha stages of opam 2.4.0. Once those stages successfully passed i think we should be able to release patch.3.0.0 around mid to late April.

@kit-ty-kate kit-ty-kate force-pushed the opam-repository-fixes branch from 4c6b846 to f68daa3 Compare February 14, 2025 14:04
@kit-ty-kate kit-ty-kate marked this pull request as ready for review February 14, 2025 14:05
@kit-ty-kate kit-ty-kate requested a review from hannesm February 14, 2025 14:06
@kit-ty-kate kit-ty-kate changed the title Parse and apply all the patch files from opam-repository Parse and apply all the unified patch files from opam-repository Feb 14, 2025
@kit-ty-kate kit-ty-kate force-pushed the opam-repository-fixes branch 2 times, most recently from 88247d9 to 06c88d5 Compare February 14, 2025 20:06
@kit-ty-kate kit-ty-kate marked this pull request as draft February 15, 2025 03:24
Examples of this behaviour can be seen in configure.diff from stone.0.1
Examples of this behaviour can be seen in easy-format-make.diff from easy-format.1.0.1
…gorithm

Examples of this behaviour can be seen in config.patch from afl-persistent.1.4
Examples of this behaviour can be seen in fix_404.patch from bitstring.2.0.4
and unsafe_string.patch from ordma.0.0.5
This follows the GNU patch behaviour, but we're loosing support
for filenames containing spaces when parsing busybox diffs.
@kit-ty-kate kit-ty-kate force-pushed the opam-repository-fixes branch from cfc167d to 445da59 Compare February 22, 2025 02:37
@kit-ty-kate kit-ty-kate marked this pull request as ready for review February 22, 2025 02:37
@hannesm
Copy link
Owner

hannesm commented Mar 6, 2025

I reviewed commit-by-commit, and am happy about your PR. Please feel free to merge & release.

@kit-ty-kate
Copy link
Collaborator Author

Thanks for the review!

@kit-ty-kate kit-ty-kate merged commit 4ed8475 into hannesm:main Mar 6, 2025
1 check failed
@kit-ty-kate kit-ty-kate deleted the opam-repository-fixes branch March 6, 2025 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants