-
Notifications
You must be signed in to change notification settings - Fork 371
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
Internal repos-config
new syntax
#6393
base: master
Are you sure you want to change the base?
Conversation
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'd be interested in a crash course on OpamPp
at the next meeting to be able to understand the new Repos_configSyntax
module better but overall otherwise that looks fine to me.
We might also want to discuss removing the soft/hard upgrade distinction.
(fun ~pos:_ repos -> | ||
let repos, errs = List.split repos in | ||
let repos = List.filter_map Fun.id repos in | ||
let map = OpamRepositoryName.Map.of_list repos in | ||
let errs = List.flatten errs in | ||
map, errs) | ||
(fun (map, _errs) -> | ||
OpamRepositoryName.Map.bindings map | ||
|> List.map (fun x -> Some x, [])) |
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.
These two functions can be done more efficiently in one fold each
(List.map (fun (_, r, u) -> r, (u,None)) prio_repositories)); | ||
(List.map (fun (_, repo_name, repoc_url) -> | ||
repo_name, | ||
OpamFile.Repos_config.{repoc_url; repoc_trust = None}) |
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.
It's probably best to use the proper naming syntax instead of the local open one to avoid confusion in the future between values and fields:
OpamFile.Repos_config.{repoc_url; repoc_trust = None}) | |
{OpamFile.Repos_config.repoc_url; repoc_trust = None}) |
let OpamFile.Repos_config_Legacy.{repoc_url; repoc_trust} = | ||
old_repo | ||
in | ||
OpamFile.Repos_config.{repoc_url; repoc_trust}) |
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.
let OpamFile.Repos_config_Legacy.{repoc_url; repoc_trust} = | |
old_repo | |
in | |
OpamFile.Repos_config.{repoc_url; repoc_trust}) | |
let {OpamFile.Repos_config_Legacy.repoc_url; repoc_trust} = | |
old_repo | |
in | |
{OpamFile.Repos_config.repoc_url; repoc_trust}) |
@@ -164,10 +164,10 @@ let load lock_kind gt = | |||
load with best-effort (read-only)" | |||
(OpamVersion.to_string (OpamFile.Config.opam_root_version gt.config)) | |||
(OpamVersion.to_string (OpamFile.Config.root_version)); | |||
let mk_repo name (url, ta) = { | |||
let mk_repo name OpamFile.Repos_config.{repoc_url; repoc_trust} = { |
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.
let mk_repo name OpamFile.Repos_config.{repoc_url; repoc_trust} = { | |
let mk_repo name {OpamFile.Repos_config.repoc_url; repoc_trust} = { |
@@ -280,7 +280,9 @@ let write_config rt = | |||
OpamFile.Repos_config.write (OpamPath.repos_config rt.repos_global.root) | |||
(OpamRepositoryName.Map.filter_map (fun _ r -> | |||
if r.repo_url = OpamUrl.empty then None | |||
else Some (r.repo_url, r.repo_trust)) | |||
else | |||
Some OpamFile.Repos_config.{ repoc_url = r.repo_url; |
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.
Some OpamFile.Repos_config.{ repoc_url = r.repo_url; | |
Some { OpamFile.Repos_config.repoc_url = r.repo_url; |
(OpamFile.make | ||
(OpamFile.filename f))); |
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.
(OpamFile.make | |
(OpamFile.filename f))); | |
(OpamFile.make (OpamFile.filename f))); |
|
||
# Packages matching: any | ||
# No matches found | ||
### sh -c "rm OPAM/repo/state-*.cache" | ||
### opam list -A | ||
[WARNING] Errors in ${BASEDIR}/OPAM/repo/repos-config, some fields have been ignored: | ||
- At ${BASEDIR}/OPAM/repo/repos-config:2:16-2:23:: | ||
expected url | ||
- In ${BASEDIR}/OPAM/repo/repos-config: |
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.
what is the reason we're loosing the position here?
@@ -1386,7 +1386,7 @@ module ConfigSyntax = struct | |||
let internal = "config" | |||
let format_version = OpamVersion.of_string "2.1" | |||
let file_format_version = OpamVersion.of_string "2.0" | |||
let root_version = OpamVersion.of_string "2.2" | |||
let root_version = OpamVersion.of_string "2.4" |
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.
let root_version = OpamVersion.of_string "2.4" | |
let root_version = OpamVersion.of_string "2.4~alpha1" |
It is quite difficult and misleading to repository related information in the
repos-config
file (like in #4933 and #6283).The current format contain a list with a some optional elements, which complicated the parsing
The PR implements a new syntax for this file: each repository is a section with it's own defined labeled fields:
Adding then a new field is quite easy.
This change implies an hard upgrade (no best effort loading if read only) to update the opam root.
TODO: