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

opam root version: fix upgrade from a non global config change #5305

Merged
merged 4 commits into from
Feb 28, 2023

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Sep 30, 2022

No description provided.

@rjbou rjbou added PR: QUEUED Pending pull request, waiting for other work to be merged or closed PR: WIP Not for merge at this stage labels Sep 30, 2022
@rjbou rjbou added this to the 2.2.0~alpha milestone Sep 30, 2022
@rjbou rjbou added PR: WAITING FOR REVIEW and removed PR: WIP Not for merge at this stage labels Oct 13, 2022
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Oct 14, 2022
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
@rjbou rjbou force-pushed the root-fx branch 2 times, most recently from 0efc815 to afbcb56 Compare January 27, 2023 16:29
@rjbou rjbou mentioned this pull request Jan 27, 2023
1 task
@rjbou rjbou added PR: WIP Not for merge at this stage and removed PR: WAITING FOR REVIEW labels Feb 9, 2023
@rjbou rjbou added PR: NEEDS UPDATE and removed PR: WIP Not for merge at this stage labels Feb 9, 2023
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

We're nearly there with this thorny thing! I have various notes to put back into the wiki on the root versions stuff.

master_changes.md Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
@@ -1760,25 +1760,44 @@ end
module Repos_configSyntax = struct

let internal = "repos-config"
let format_version = OpamVersion.of_string "2.0"
let format_version = OpamVersion.of_string "2.1"
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to bump it straight to 2.2, wouldn't it? (I think the "gap" is less confusing than having a 2.1 version of the format which technically can't be read by opam 2.1's libraries!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't remember what we say about those versions : should they follow opam versions ?

Copy link
Member

Choose a reason for hiding this comment

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

I think the intention was that we'd use opam versions when it was bumped, yes - so it just so happened that opam 2.1 uses opam 2.0's repo config format

src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamFormatUpgrade.ml Outdated Show resolved Hide resolved
src/state/opamStateTypes.mli Outdated Show resolved Hide resolved
@rjbou
Copy link
Collaborator Author

rjbou commented Feb 15, 2023

Updated

Used to retrieve information about remaining upgrades to do (repo or
switch)
When root is bumped but only repo or switch is modified, upgraded files
can be written without writing the root bump (on-the-fly global state
upgrade).
Copy link
Member

@dra27 dra27 left a comment

Choose a reason for hiding this comment

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

LGTM! (sorry for the delay re-reviewing)

@dra27 dra27 merged commit 88002fb into ocaml:master Feb 28, 2023
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