From 533146787713a844748e935a389879044737218b Mon Sep 17 00:00:00 2001 From: Kate Date: Tue, 18 Jun 2024 12:54:16 +0100 Subject: [PATCH 1/2] reftest: add a complete test to make sure effectively_equal does not take the location of the fields into account --- master_changes.md | 1 + tests/reftests/dune.inc | 18 ++ tests/reftests/effectively-equal.test | 227 ++++++++++++++++++++++++++ 3 files changed, 246 insertions(+) create mode 100644 tests/reftests/effectively-equal.test diff --git a/master_changes.md b/master_changes.md index 26f3b2e5a8a..ced8e49a992 100644 --- a/master_changes.md +++ b/master_changes.md @@ -99,6 +99,7 @@ users) ## Reftests ### Tests + * add a complete test to make sure effectively_equal does not take the location of the fields into account [#6029 @kit-ty-kate] ### Engine diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index ccc8e3b818d..4b78e8ba8ca 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -416,6 +416,24 @@ %{targets} (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:download.test} %{read-lines:testing-env})))) +(rule + (alias reftest-effectively-equal) + (action + (diff effectively-equal.test effectively-equal.out))) + +(alias + (name reftest) + (deps (alias reftest-effectively-equal))) + +(rule + (targets effectively-equal.out) + (deps root-N0REP0) + (package opam) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:effectively-equal.test} %{read-lines:testing-env})))) + (rule (alias reftest-empty-conflicts-001) (action diff --git a/tests/reftests/effectively-equal.test b/tests/reftests/effectively-equal.test new file mode 100644 index 00000000000..765a58f1937 --- /dev/null +++ b/tests/reftests/effectively-equal.test @@ -0,0 +1,227 @@ +N0REP0 +### : test all the fields to check that effectively_equal works as expected +### : each fields' value is placed on the next line to ensure the position will differ from the normalised version +### +opam-version: +"2.0" +name: +"test-all-fields" +version: +"1" +depends: +["dummy-dep" {some-filter}] +conflicts: +["dummy-conflict" {some-filter}] +depopts: +["dummy-dep" {some-filter}] +conflict-class: +"conflict-class" +available: +true +flags: +[plugin] +setenv: +[ENV = "some value"] +build: +["false" {some-filter}] +run-test: +["false" {some-filter}] +install: +["false" {some-filter}] +remove: +["false" {some-filter}] +substs: +["some-file"] +patches: +["some-patches" {some-filter}] +build-env: +[BUILD_ENV = "some value"] +extra-source +"some-file" +{ +src: +"https://example.com" +checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +messages: +["message" {some-filter}] +post-messages: +["post-message" {some-filter}] +depexts: +[["depext"] {some-filter}] +dev-repo: +"git+https://example.com" +pin-depends: +["dep.dev" "https://example.com"] +maintainer: +"maintainer" +authors: +["author1" "author2"] +license: +"LicenseRef-custom" +tags: +["tag"] +homepage: +"https://example.com" +doc: +"https://example.com" +bug-reports: +"https://example.com" +x-some-extension: +[ext "something"] +x-env-path-rewrite: +[[ENV false]] +url +{ +src: "https://example.com" +checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +synopsis: +"some synopsis" +description: +"some description" +### opam switch create eff-eq --empty +### opam var --global some-filter=false +Added '[some-filter "false" "Set through 'opam var'"]' to field global-variables in global configuration +### opam install --fake test-all-fields +The following actions will be faked: +=== install 1 package + - install test-all-fields 1 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Faking installation of test-all-fields.1 +Done. +### cat OPAM/repo/default/packages/test-all-fields/test-all-fields.1/opam +opam-version: +"2.0" +name: +"test-all-fields" +version: +"1" +depends: +["dummy-dep" {some-filter}] +conflicts: +["dummy-conflict" {some-filter}] +depopts: +["dummy-dep" {some-filter}] +conflict-class: +"conflict-class" +available: +true +flags: +[plugin] +setenv: +[ENV = "some value"] +build: +["false" {some-filter}] +run-test: +["false" {some-filter}] +install: +["false" {some-filter}] +remove: +["false" {some-filter}] +substs: +["some-file"] +patches: +["some-patches" {some-filter}] +build-env: +[BUILD_ENV = "some value"] +extra-source +"some-file" +{ +src: +"https://example.com" +checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +messages: +["message" {some-filter}] +post-messages: +["post-message" {some-filter}] +depexts: +[["depext"] {some-filter}] +dev-repo: +"git+https://example.com" +pin-depends: +["dep.dev" "https://example.com"] +maintainer: +"maintainer" +authors: +["author1" "author2"] +license: +"LicenseRef-custom" +tags: +["tag"] +homepage: +"https://example.com" +doc: +"https://example.com" +bug-reports: +"https://example.com" +x-some-extension: +[ext "something"] +x-env-path-rewrite: +[[ENV false]] +url +{ +src: "https://example.com" +checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +synopsis: +"some synopsis" +description: +"some description" +### cat OPAM/eff-eq/.opam-switch/packages/test-all-fields.1/opam +opam-version: "2.0" +synopsis: "some synopsis" +description: "some description" +maintainer: "maintainer" +authors: ["author1" "author2"] +license: "LicenseRef-custom" +tags: "tag" +homepage: "https://example.com" +doc: "https://example.com" +bug-reports: "https://example.com" +depends: [ + "dummy-dep" {some-filter} +] +depopts: [ + "dummy-dep" {some-filter} +] +conflicts: [ + "dummy-conflict" {some-filter} +] +conflict-class: "conflict-class" +flags: plugin +setenv: ENV = "some value" +build: "false" {some-filter} +run-test: "false" {some-filter} +install: "false" {some-filter} +remove: "false" {some-filter} +substs: "some-file" +patches: "some-patches" {some-filter} +build-env: BUILD_ENV = "some value" +messages: "message" {some-filter} +post-messages: "post-message" {some-filter} +depexts: ["depext"] {some-filter} +dev-repo: "git+https://example.com" +pin-depends: ["dep.dev" "https://example.com"] +url { + src: "https://example.com" + checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +extra-source "some-file" { + src: "https://example.com" + checksum: "md5=d41d8cd98f00b204e9800998ecf8427e" +} +x-env-path-rewrite: [ + [ENV false] +] +x-some-extension: [ext "something"] +### opam upgrade +Already up-to-date. +Nothing to do. +### rm OPAM/eff-eq/.opam-switch/packages/cache +### opam upgrade --show +The following actions would be performed: +=== recompile 1 package + - recompile test-all-fields 1 [upstream or system changes] From 764f2820fe3c6809bb05ad951a5e2086296eae50 Mon Sep 17 00:00:00 2001 From: Kate Date: Tue, 18 Jun 2024 12:01:29 +0100 Subject: [PATCH 2/2] Nullify positions of the extensions fields in OpamFile.OPAM.effective_part --- master_changes.md | 2 ++ src/format/opamFile.ml | 5 ++- src/format/opamTypesBase.ml | 50 +++++++++++++++++++++++++++ src/format/opamTypesBase.mli | 2 ++ tests/reftests/effectively-equal.test | 5 ++- 5 files changed, 60 insertions(+), 4 deletions(-) diff --git a/master_changes.md b/master_changes.md index ced8e49a992..e1352844fb6 100644 --- a/master_changes.md +++ b/master_changes.md @@ -40,6 +40,7 @@ users) ## Var/Option ## Update / Upgrade + * Fix `opam upgrade` wanting to recompile opam files containing the `x-env-path-rewrite` field [#6029 @kit-ty-kate - fix #6028] ## Tree @@ -119,5 +120,6 @@ users) ## opam-solver ## opam-format + * `OpamTypesBase`: Add `nullify_pos_map` and `nullify_pos_value` [#6029 @kit-ty-kate] ## opam-core diff --git a/src/format/opamFile.ml b/src/format/opamFile.ml index 0c0f53b16a9..87e719e1716 100644 --- a/src/format/opamFile.ml +++ b/src/format/opamFile.ml @@ -3629,7 +3629,10 @@ module OPAM = struct (* We keep only `x-env-path-rewrite` as it affects build/install *) extensions = - OpamStd.String.Map.filter (fun x _ -> String.equal rewrite_xfield x) + OpamStd.String.Map.filter_map (fun k v -> + if String.equal rewrite_xfield k + then Some (OpamTypesBase.nullify_pos_value v) + else None) t.extensions; url = OpamStd.Option.map effective_url t.url; diff --git a/src/format/opamTypesBase.ml b/src/format/opamTypesBase.ml index eb016bc0bb1..e8f252fdf2d 100644 --- a/src/format/opamTypesBase.ml +++ b/src/format/opamTypesBase.ml @@ -62,6 +62,56 @@ let pos_null = stop = -1, -1; } let nullify_pos pelem = {pelem; pos = pos_null} +let nullify_pos_map f {pelem; pos = _} = nullify_pos (f pelem) + +let rec nullify_pos_value {pelem; pos = _} = + nullify_pos @@ + match pelem with + | Bool b -> Bool (b : bool) + | Int i -> Int (i : int) + | String s -> String (s : string) + | Relop (relop, v1, v2) -> + Relop + (nullify_pos_map + (fun (x : OpamParserTypes.FullPos.relop_kind) -> x) + relop, + nullify_pos_value v1, + nullify_pos_value v2) + | Prefix_relop (relop, v) -> + Prefix_relop + (nullify_pos_map + (fun (x : OpamParserTypes.FullPos.relop_kind) -> x) + relop, + nullify_pos_value v) + | Logop (logop, v1, v2) -> + Logop + (nullify_pos_map + (fun (x : OpamParserTypes.FullPos.logop_kind) -> x) + logop, + nullify_pos_value v1, + nullify_pos_value v2) + | Pfxop (pfxop, v) -> + Pfxop + (nullify_pos_map + (fun (x : OpamParserTypes.FullPos.pfxop_kind) -> x) + pfxop, + nullify_pos_value v) + | Ident s -> Ident (s : string) + | List {pelem = l; pos = _} -> + List (nullify_pos (List.map nullify_pos_value l)) + | Group {pelem = l; pos = _} -> + Group (nullify_pos (List.map nullify_pos_value l)) + | Option (v, {pelem = filter; pos = _}) -> + Option + (nullify_pos_value v, + nullify_pos (List.map nullify_pos_value filter)) + | Env_binding (v1, env_update_op, v2) -> + Env_binding + (nullify_pos_value v1, + nullify_pos_map + (fun (x : OpamParserTypes.FullPos.env_update_op_kind) -> x) + env_update_op, + nullify_pos_value v2) (* XXX update *) let pos_best pos1 pos2 = diff --git a/src/format/opamTypesBase.mli b/src/format/opamTypesBase.mli index 8ba6da9f23d..11b5478c58b 100644 --- a/src/format/opamTypesBase.mli +++ b/src/format/opamTypesBase.mli @@ -42,6 +42,8 @@ val string_of_shell: shell -> string (** The empty file position *) val pos_null: pos val nullify_pos : 'a -> 'a with_pos +val nullify_pos_map : ('a -> 'b) -> 'a with_pos -> 'b with_pos +val nullify_pos_value : value -> value (** [pos_best pos1 pos2] returns the most detailed position between [pos1] and [pos2] (defaulting to [pos1]) *) diff --git a/tests/reftests/effectively-equal.test b/tests/reftests/effectively-equal.test index 765a58f1937..80743d62aca 100644 --- a/tests/reftests/effectively-equal.test +++ b/tests/reftests/effectively-equal.test @@ -222,6 +222,5 @@ Already up-to-date. Nothing to do. ### rm OPAM/eff-eq/.opam-switch/packages/cache ### opam upgrade --show -The following actions would be performed: -=== recompile 1 package - - recompile test-all-fields 1 [upstream or system changes] +Already up-to-date. +Nothing to do.