Skip to content

fix(subst): subst for opam files in opam/ subdir #9895

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.

Sign up for GitHub

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

Merged
rgrinberg merged 2 commits into from
Feb 8, 2024
Merged

fix(subst): subst for opam files in opam/ subdir #9895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8886b25
fix(subst): handle opam files in opam directory
emillon Feb 1, 2024
fce4761
refactor: [Vcs.files] should return source paths
rgrinberg Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions bin/subst.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
open Import

let is_path_a_source_file path =
match Path.extension path with
match Path.extension (Path.source path) with
| ".flv"
| ".gif"
| ".ico"
Expand All @@ -19,7 +19,7 @@ let is_path_a_source_file path =
;;

let is_kind_a_source_file path =
match Path.stat path with
match Path.stat (Path.source path) with
| Ok st -> st.st_kind = S_REG
| Error (ENOENT, "stat", _) ->
(* broken symlink *)
Expand Down Expand Up @@ -119,8 +119,8 @@ let subst_string s path ~map =
Some (Buffer.contents buf)
;;

let subst_file path ~map =
match Io.with_file_in path ~f:Io.read_all_unless_large with
let subst_file path ~map opam_package_files =
match Io.with_file_in (Path.source path) ~f:Io.read_all_unless_large with
| Error () ->
let hints =
if Sys.word_size = 32
Expand All @@ -131,13 +131,16 @@ let subst_file path ~map =
]
else []
in
User_warning.emit ~hints [ Pp.textf "Ignoring large file: %s" (Path.to_string path) ]
User_warning.emit
~hints
[ Pp.textf "Ignoring large file: %s" (Path.Source.to_string path) ]
| Ok s ->
let s =
if Path.is_root (Path.parent_exn path) && Package.is_opam_file path
if Path.Source.Set.mem opam_package_files path
then "version: \"%%" ^ "VERSION_NUM" ^ "%%\"\n" ^ s
else s
in
let path = Path.source path in
(match subst_string s ~map path with
| None -> ()
| Some s -> Io.write_file path s)
Expand All @@ -161,7 +164,7 @@ module Dune_project = struct
; project : Dune_project.t
}

let filename = Path.in_source Dune_project.filename
let filename = Path.Source.of_string Dune_project.filename

let load ~dir ~files ~infer_from_opam_files =
let open Memo.O in
Expand Down Expand Up @@ -243,8 +246,8 @@ module Dune_project = struct
replace_text !ofs !ofs version_field)
else replace_text !ofs !ofs ("\n" ^ version_field))
in
let s = Option.value (subst_string s ~map filename) ~default:s in
if s <> t.contents then Io.write_file filename s
let s = Option.value (subst_string s ~map (Path.source filename)) ~default:s in
if s <> t.contents then Io.write_file (Path.source filename) s
;;
end

Expand Down Expand Up @@ -306,6 +309,7 @@ let subst vcs =
(let files =
(* Filter-out files form sub-directories *)
List.fold_left files ~init:String.Set.empty ~f:(fun acc fn ->
let fn = Path.source fn in
if Path.is_root (Path.parent_exn fn)
then String.Set.add acc (Path.to_string fn)
else acc)
Expand Down Expand Up @@ -384,9 +388,14 @@ let subst vcs =
in
let watermarks = make_watermark_map ~commit ~version ~dune_project ~info in
Dune_project.subst ~map:watermarks ~version dune_project;
let opam_package_files =
Dune_project.packages dune_project.project
|> Package.Name.Map.fold ~init:Path.Source.Set.empty ~f:(fun package acc ->
Path.Source.Set.add acc (Package.opam_file package))
in
List.iter files ~f:(fun path ->
if is_a_source_file path && not (Path.equal path Dune_project.filename)
then subst_file path ~map:watermarks)
if is_a_source_file path && not (Path.Source.equal path Dune_project.filename)
then subst_file path ~map:watermarks opam_package_files)
;;

let subst () =
Expand Down
2 changes: 2 additions & 0 deletions doc/changes/9895-subst-opam.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- subst: correctly handle opam files in opam/ subdirectory
(#9895, fixes #9862, @emillon)
2 changes: 1 addition & 1 deletion src/dune_vcs/vcs.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ let files =
let f run args t =
let open Fiber.O in
let+ l = run t args in
List.map l ~f:Path.in_source
List.map l ~f:Path.Source.of_string
in
Staged.unstage
@@ make_fun
Expand Down
2 changes: 1 addition & 1 deletion src/dune_vcs/vcs.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ val describe : t -> string option Memo.t
val commit_id : t -> string option Memo.t

(** List of files committed in the repo *)
val files : t -> Path.t list Memo.t
val files : t -> Path.Source.t list Memo.t

(** VCS commands *)
val git : Path.t Lazy.t
Expand Down
4 changes: 3 additions & 1 deletion test/blackbox-tests/test-cases/subst/opam-files/in-opam-subdir.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ See #9862.
> (lang dune 3.8)
> (name pkg)
> (opam_file_location inside_opam_directory)
> (package (name pkg))
> EOF

$ mkdir opam
Expand All @@ -17,7 +18,7 @@ Git setup is required for dune subst:
$ git init -q
$ git add dune-project opam/pkg.opam
$ git commit -m message|grep -v root-commit
2 files changed, 4 insertions(+)
2 files changed, 5 insertions(+)
create mode 100644 dune-project
create mode 100644 opam/pkg.opam
$ git tag -a 1.2.3 -m 'tag message'
Expand All @@ -27,4 +28,5 @@ Git setup is required for dune subst:
Subst adds a version number based on the git commit:

$ cat opam/pkg.opam
version: "1.2.3"
opam-version: "2.0"