Skip to content

Conversation

@hannesm
Copy link
Member

@hannesm hannesm commented May 22, 2024

this is step 1 for ocaml/opam-repository#25876

naming conventions patches/package_p/filename

if filename already existed, and has a different checksum, I put a .N at the end.

the result is pretty good deduplication :)

let me know what you think about it, or whether you like to see the corresponding changes for opam-repository!?

EDIT: I did this in a two-step process, for the sake of usability I didn't use any .N filenames anymore.

So, a first iteration was to figure out which files can be deduplicated, a second did exclude the files that are not safe to be deduplicated from the set.

The naming is "patches//filename" if the filename is unique over all versions and all versions use the identical file. Otherwise, the name is "package//filename.package-version".

The corresponding opam-repository change is at https://github.com/hannesm/opam-repository/tree/extra-file-to-extra-source (if you like to see the commit, hannesm/opam-repository@fde76a6 is it).

@hannesm

This comment was marked as outdated.

@hannesm

This comment was marked as outdated.

@hannesm
Copy link
Member Author

hannesm commented May 23, 2024

if anyone has further ideas / remarks about naming, I'm happy to integrate whatever you like. since this was discussed yesterday in the opam-repository meeting, I'm cc'ing @shonfeder @raphael-proust @kit-ty-kate -- the context is ocaml/opam-repository#25876

@hannesm
Copy link
Member Author

hannesm commented May 23, 2024

I updated the commit, and the description about naming. For what it is worth, here's the list of files that keep their package-version suffix:

"META"
"FrontC/opam.patch" ;
"abella/abella.install" ;
"afl/add-uninstall-target.patch" ;
"alt-ergo/alt-ergo.install" ;
"altgr-ergo/altgr-ergo.install" ;
"atd/atd.install" ;
"atdgen/atdgen.install" ;
"beluga/beluga.install" ;
"biniou/biniou.install" ;
"bisect/opam.patch" ;
"bisect_ppx/bisect_ppx.install" ;
"bolt/opam.patch" ;
"bsdowl/remove.sh" ;
"camlhighlight/sexp-dir.patch.in" ;
"camlidl/camlidl.install" ;
"camlp5/camlp5.install" ;
"camlzip/build_with_trunk.patch" ;
"camlzip/fix-install.patch" ;
"cca/cca.install" ;
"conf-bap-llvm/find-llvm.ml.in" ;
"conf-binutils/find-binutils.ml.in" ;
"conf-gmp-powm-sec/test.c" ;
"conf-gmp/test.c" ;
"conf-ida/find-ida.ml.in" ;
"conf-libclang/configure.sh" ;
"conf-libev/discover.ml" ;
"conf-libssl/homebrew.sh" ;
"conf-llvm/configure.sh" ;
"conf-openblas/test.c" ;
"coq/CAML_LD_LIBRARY_PATH.patch" ;
"coq/configure.patch" ;
"coq/coq.install" ;
"coq/disable_warn_70.patch" ;
"coqide/MAKEFILE_remove_useless_for_coqide.patch" ;
"coqide/coqide.install" ;
"core/core.install" ;
"core/fix_META.patch" ;
"core_extended/fix_META.patch" ;
"core_extended/openbsd-quota-disable.diff" ;
"csv/csv.install" ;
"eigen/eigen.install" ;
"eliom/eliom.install" ;
"frama-c/frama-c.install" ;
"gmp-freestanding/gmp-freestanding.pc" ;
"gmp-freestanding/mirage-build.sh" ;
"gmp-xen/mirage-build.sh" ;
"gpr/gpr.install" ;
"iocaml/iocaml.install" ;
"iocamljs-kernel/iocamljs-kernel.install" ;
"js_of_ocaml/js_of_ocaml.install" ;
"lablgtk/lablgldir.patch" ;
"lablgtk3/lablgtk.install" ;
"lambda-term/lambda-term.install" ;
"lambdoc/lambdoc.install" ;
"ledit/ledit.install" ;
"libres3/libres3.install" ;
"llvm/META.patch" ;
"llvm/fix-macos.patch" ;
"llvm/fix-rhel.patch" ;
"llvm/fix-shared.patch" ;
"llvm/install.sh" ;
"llvm/link-META.patch" ;
"llvm/link.patch" ;
"lwt/lwt.install" ;
"menhir/menhir.install" ;
"mirage-www/mirage-www.install" ;
"mldonkey/mldonkey.install" ;
"mlmpfr/mlmpfr_compatibility_test.c" ;
"mlmpfr/test.c" ;
"modelica_ml/modelica.ml.install" ;
"mpp/mpp.install" ;
"msgpack/no-camlp4.patch" ;
"nocrypto/werror.patch" ;
"num/findlib-install.patch" ;
"oasis/oasis.install" ;
"oasis2opam/oasis2opam.install" ;
"ocaml-base-compiler/PIC.patch" ;
"ocaml-base-compiler/alt-signal-stack.patch" ;
"ocaml-base-compiler/fix-gcc10.patch" ;
"ocaml-base-compiler/pr4867.patch" ;
"ocaml-config/gen_ocaml_config.ml.in" ;
"ocaml-variants/add-conditional-compilation.patch" ;
"ocaml-variants/fix-gcc10.patch" ;
"ocamlfind/no-awk-check.patch" ;
"ocamlfind/ocamlfind.install" ;
"ocamlfind/termux.patch" ;
"ocamlmod/ocamlmod.install" ;
"ocp-indent/fix-warn-error.patch" ;
"ocp-index/ocaml.4.02.patch" ;
"ocp-index/ocaml.4.03.patch" ;
"ocsigenserver/fix-gmake-4.3.patch" ;
"omd/omd.install" ;
"oranger/compile_ranger.sh" ;
"ospec/ospec.install" ;
"ott/ott.install" ;
"rdbg/rdbg.install" ;
"stog/stog.install" ;
"unison/ocaml48.patch" ;
"unison/unison.install" ;
"utop/utop.install" ;
"wall/fix-ocaml-beta.patch" ;
"why3/Makefile.patch" ;
"why3/why3.install" ;
"zarith-freestanding/mirage-build.sh" ;
"zarith-freestanding/mirage-install.sh" ;
"zarith-freestanding/mirage-uninstall.sh" ;
"zarith-freestanding/no-dynlink.patch" ;
"zarith-xen/mirage-install.sh" ;
"zarith/z_pp.pl.patch" ;

@hannesm
Copy link
Member Author

hannesm commented May 23, 2024

went ahead and cleaned up the ocaml-variants vs ocaml-base-compiler stuff a bit, fewer files, ready to be merged now

@mseri
Copy link
Member

mseri commented May 23, 2024

Thanks for the thoughts and efforts you put into this. The naming scheme sounds good. I am sad we did not do something similar for the sources sources/pkg-name/version.tar.gz (on our defense this was supposed to be very temporary solution – I'll never make that assumption again 😅)

@mseri
Copy link
Member

mseri commented May 23, 2024

This looks great to me. I'll wait for @raphael-proust, @kit-ty-kate or @shonfeder to have a look since I was not at the meeting

@hannesm
Copy link
Member Author

hannesm commented May 23, 2024

Thanks for the thoughts and efforts you put into this. The naming scheme sounds good. I am sad we did not do something similar for the sources sources/pkg-name/version.tar.gz (on our defense this was supposed to be very temporary solution – I'll never make that assumption again 😅)

This is not in scope for this PR, but if that is desired, I suggest to first do a copy of the tarballs to the respective target location, then modify opam-repository with the new urls, and then remove the original location of the tarballs. Since it is git, a copy shouldn't take up much space.

@mseri
Copy link
Member

mseri commented May 23, 2024 via email

@kit-ty-kate
Copy link
Member

It may break people with their own copies of the repo or stuck to older versions

in what way?

@raphael-proust
Copy link
Contributor

The naming scheme sounds good to me. I've looked at a couple of files but I'm not sure how to review this MR. I guess one way to do it would be to try to generate the same work and check for diff. Do you have another suggestion?

@hannesm
Copy link
Member Author

hannesm commented May 23, 2024

FWIW, I didn't do manual work, but used opam admin migrate-extrafiles from https://github.com/hannesm/opam/tree/migrate-extra-files at hannesm/opam@b9ea01d. The last commit (to condense the fix-gcc patches) was manual labor. I don't think anyone should redo the work manually -- but of course you can review the opam admin subcommand or re-run it :)

@mseri mseri merged commit 7d49455 into ocaml:main May 29, 2024
@hannesm hannesm deleted the add-patches branch May 29, 2024 13:47
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.

4 participants