-
Notifications
You must be signed in to change notification settings - Fork 371
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
new subcommand: opam admin migrate-extrafiles #5960
base: master
Are you sure you want to change the base?
Conversation
Thanks for the PR! We'll look at it once opam 2.2 release is done |
For the future reviewer, a slightly modified piece (see hannesm/opam@migrate...hannesm:opam:migrate-extra-files) was used for driving the rewrite for opam-repository (ocaml/opam-repository#25960) The main differences are:
So, it boils down to what to expect from such an automated migration, and whether there's need to have the result conveniently editable by people (or by scripts). Since some choices are hardcoded (such as the hash algorithm -- the tool has only ever been applied with SHA256 (and there are special code paths for "what if an MD5 is present" and "what to do if a SHA512 is present"), which is unlikely something you'd want to embed into the opam source tree. About file naming, for deduplication reasons (and please note, there are other ways to achieve deduplication), the layout on the opam-source-archives repository is "patches/pkg-name/file1..N", i.e. not another subdirectory for pkg_version. |
This subcommand allows to move all extra-files of an existing opam repository into extra-sources. The files are extracted into a specified local directory, and the opam files are edited in-place.
…min-topics.inc files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've rebased and made some improvements to the UI of the command but i haven't reviewed the core of the code yet, although it looks fine on first read.
* some indent * take an url argument as prefix * work with dir/url types instead of string concatenation with dir sep * add test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a commit with some modification, to review.
Note that there is no requirement on the already present extrafiles, no checksum check done, etc. We may not want to fail in case an extra file is not declared in the opam file, but the checksum mismatch should be notified at least.
@@ -38,5 +38,5 @@ let () = | |||
\ (section man)\n\ | |||
\ (package opam)\n\ | |||
\ (files%s))\n" | |||
(String.concat " " | |||
(String.concat "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be extracted to its own PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a PR sounds a little excessive for such an internal change. What about its own commit?
Co-authored-by: R. Boujbel <rjbou@ocamlpro.com>
This subcommand allows to move all extra-files of an existing opam repository into extra-sources. The files are extracted into a specified local directory, and the opam files are edited in-place.
see #5811 for motivation