Skip to content

Internal repos-config new syntax#6393

Open
rjbou wants to merge 5 commits intoocaml:masterfrom
rjbou:new-repos-config
Open

Internal repos-config new syntax#6393
rjbou wants to merge 5 commits intoocaml:masterfrom
rjbou:new-repos-config

Conversation

@rjbou
Copy link
Copy Markdown
Collaborator

@rjbou rjbou commented Feb 20, 2025 •
edited by kit-ty-kate
Loading

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

repositories: [ <string> { <URL> } { {<string>}+ } { <int> } ... ]: lists the configured repository idents, their URLs and trust anchors. The format is similar to repositories: from opamrc, except that the itself is optional.

The PR implements a new syntax for this file: each repository is a section with it's own defined labeled fields:

repo <string> {
  url: <URL>
  fingerprints:  <string>
  quorum: <int>
}

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.

fix #6327

TODO:

@rjbou rjbou requested review from dra27 and kit-ty-kate February 20, 2025 19:02
@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Feb 20, 2025
@rjbou rjbou mentioned this pull request Feb 21, 2025
Open
kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate left a 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.

src/format/opamFile.ml Outdated
Comment on lines +2076 to +2084
(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, []))
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate Feb 21, 2025 •
edited
Loading

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

tests/reftests/repository.test
[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:
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate Feb 21, 2025

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?

Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpamFormat.section does not keep positions

Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the new version, there is also the same issue, it is because the error is checked after parsing errors.

kit-ty-kate
kit-ty-kate reviewed Feb 21, 2025
View reviewed changes
@kit-ty-kate kit-ty-kate linked an issue Feb 21, 2025 that may be closed by this pull request
Update internal repos-config file format #6327
Open
@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Mar 12, 2025
@rjbou rjbou force-pushed the new-repos-config branch from 0e49b8f to 732f755 Compare March 12, 2025 18:28
@rjbou rjbou marked this pull request as ready for review March 12, 2025 18:28
rjbou
rjbou commented Mar 12, 2025
View reviewed changes
@kit-ty-kate kit-ty-kate mentioned this pull request Mar 12, 2025
Merged
@rjbou rjbou modified the milestones: 2.4.0~alpha1, 2.5.0~alpha1 Apr 7, 2025
@kit-ty-kate kit-ty-kate added PR: NEEDS UPDATE and removed PR: QUEUED Pending pull request, waiting for other work to be merged or closed labels Sep 12, 2025
@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Sep 17, 2025

New implem, simpler, and that permit to introduce repository variables & repositories config option update via opam option/opam var.

rjbou
rjbou commented Sep 17, 2025
View reviewed changes
tests/reftests/repository.test
### # ---
### <OPAM/repo/repos-config>
opam-version: "2.0"
### opam repo --all
Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we can have no repo configured...

Copy link
Copy Markdown
Collaborator Author

@rjbou rjbou Nov 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

related to #6816

kit-ty-kate
kit-ty-kate approved these changes Sep 17, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm modulo history cleanup and the handful of comments

@rjbou rjbou self-assigned this Sep 18, 2025
@rjbou rjbou force-pushed the new-repos-config branch 2 times, most recently from 7fbfc6f to a493f44 Compare December 1, 2025 16:33
@rjbou
Copy link
Copy Markdown
Collaborator Author

rjbou commented Dec 3, 2025

Discussed in dev meeting: the windows error

Error: [m[ERROR] Another process has locked ${BASEDIR}/OPAM/lock and non blocking mode enabled

originates from #6417 (no CI was run on the examples of upgrade proposed during the review). There is an issue with opam system lock and windows to investigate.

@rjbou rjbou mentioned this pull request Dec 18, 2025
Merged
@kit-ty-kate kit-ty-kate added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Dec 18, 2025
@kit-ty-kate kit-ty-kate removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Dec 22, 2025
@kit-ty-kate
Copy link
Copy Markdown
Member

kit-ty-kate commented Dec 22, 2025

Good to go once rebased and the documentation updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kit-ty-kate kit-ty-kate kit-ty-kate approved these changes

@dra27 dra27 Awaiting requested review from dra27

Assignees

@rjbou rjbou

Labels

PR: NEEDS UPDATE PRIORITY

Projects

None yet

Milestone

2.6.0~alpha1

Development

Successfully merging this pull request may close these issues.

Update internal repos-config file format

2 participants

@rjbou @kit-ty-kate