Merge pull request #6659 from kit-ty-kate/fix-var-opamfile

Make the computation of pkg:opamfile match its specification

Author: kit-ty-kate

doc/pages/Manual.md

@@ -561,8 +561,10 @@ the package being defined.
   a hash identifying the precise package version and metadata, and that of all
   its dependencies
 - <a id="pkgvar-opamfile">`opamfile`</a>:
-  if the package is installed, path of its opam file, from opam internals,
-  otherwise not defined
+  if the package is installed or pinned it is the path to the internally
+  stored opam file, otherwise, if the package exists, it is the path to
+  a newly created opam file in a directory under the system temporary
+  directory (see `Stdlib.Filename.get_temp_dir_name`)
 
 Extra variables can be defined by any package at installation time, using a
 [`<pkgname>.config`](#lt-pkgname-gt-config) file with a

master_changes.md

@@ -48,6 +48,7 @@ users)
 ## Show
 
 ## Var/Option
+  * Make the computation of `pkg:opamfile` match its specification [#6659 @kit-ty-kate - fix #5346]
 
 ## Update / Upgrade
 
@@ -131,6 +132,8 @@ users)
   * Add a test showing that `opam upgrade` allows downgrades when necessary [#6690 @kit-ty-kate]
   * Add a test showing the behaviour of `opam tree` on local packages that happen to be already pinned [#6688 @kit-ty-kate]
   * Add a test showing the behaviour of version pins when one of the dependencies isn't up-to-date [#6691 @kit-ty-kate]
+  * Add a test showing the behaviour of pre-defined variables in command hooks [#6659 @rjbou]
+  * Add a test showing the behaviour of `opam var <pkg>:opamfile` [#6659 @kit-ty-kate]
 
 ### Engine
   * Fix gcc < 14.3 bug on mingw i686 [#6624 @kit-ty-kate]

src/state/opamPackageVar.ml

@@ -309,20 +309,20 @@ let resolve st ?opam:opam_arg ?(local=OpamVariable.Map.empty) v =
     | "dev", Some opam -> Some (bool (is_dev_package st opam))
     | "build-id", Some opam -> Option.map string (build_id st opam)
     | "opamfile", Some opam ->
-      (* Opamfile path is retrieved from overlay directory for pinned packages,
-         or from temporary repository in /tmp *)
-      let repos_roots reponame =
-        match Hashtbl.find st.switch_repos.repos_tmp reponame with
-        | lazy repo_root -> repo_root
-        | exception Not_found ->
-          OpamRepositoryPath.root st.switch_global.root reponame
-      in
-      OpamFile.OPAM.get_metadata_dir ~repos_roots opam
-      |> Option.map (fun d ->
-          OpamFilename.Op.(d//"opam")
-          |> OpamFilename.to_string
-          |> string
-        )
+      let nv = get_nv opam in
+      let ret file = Some (string (OpamFile.to_string file)) in
+      if OpamPackage.Set.mem nv st.installed then
+        ret (OpamPath.Switch.installed_opam st.switch_global.root st.switch nv)
+      else if OpamPackage.Set.mem nv st.pinned then
+        ret (OpamPath.Switch.Overlay.opam st.switch_global.root st.switch (OpamPackage.name nv))
+      else if OpamPackage.Set.mem nv st.packages then
+        let opam = OpamPackage.Map.find nv st.opams in
+        let temp_dir = OpamFilename.mk_tmp_dir () in
+        let dest = OpamFile.make OpamFilename.Op.(temp_dir // "opam") in
+        OpamFile.OPAM.write dest opam;
+        ret dest
+      else
+        None
     | _, _ -> None
   in
   let make_package_local v =

tests/reftests/dune.inc

@@ -902,6 +902,69 @@
    %{targets}
    (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:git.test} %{read-lines:testing-env}))))
 
+(rule
+ (alias reftest-hooks-variables)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (action
+  (diff hooks-variables.test hooks-variables.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (deps (alias reftest-hooks-variables)))
+
+(rule
+ (targets hooks-variables.out)
+ (deps root-N0REP0)
+ (enabled_if (and  (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:hooks-variables.test} %{read-lines:testing-env}))))
+
+(rule
+ (alias reftest-hooks-variables.unix)
+ (enabled_if (and (= %{os_type} "Unix") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (action
+  (diff hooks-variables.unix.test hooks-variables.unix.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (and (= %{os_type} "Unix") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (deps (alias reftest-hooks-variables.unix)))
+
+(rule
+ (targets hooks-variables.unix.out)
+ (deps root-N0REP0)
+ (enabled_if (and (= %{os_type} "Unix") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:hooks-variables.unix.test} %{read-lines:testing-env}))))
+
+(rule
+ (alias reftest-hooks-variables.win32)
+ (enabled_if (and (= %{os_type} "Win32") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (action
+  (diff hooks-variables.win32.test hooks-variables.win32.out)))
+
+(alias
+ (name reftest)
+ (enabled_if (and (= %{os_type} "Win32") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (deps (alias reftest-hooks-variables.win32)))
+
+(rule
+ (targets hooks-variables.win32.out)
+ (deps root-N0REP0)
+ (enabled_if (and (= %{os_type} "Win32") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))
+ (package opam)
+ (action
+  (with-stdout-to
+   %{targets}
+   (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:hooks-variables.win32.test} %{read-lines:testing-env}))))
+
 (rule
  (alias reftest-init-ocaml-eval-variables.unix)
  (enabled_if (and (= %{os_type} "Unix") (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1))))