From 75983c3daa70040192800807b5635cde2c0229bf Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Sun, 7 Aug 2022 13:16:20 +0200 Subject: [PATCH] Merge pull request #4879 from rjbou/fx Fix working dir checkout --- CHANGES | 3 + src/repository/opamDarcs.ml | 6 +- src/repository/opamGit.ml | 12 +- src/repository/opamHg.ml | 6 + src/repository/opamVCS.ml | 2 + src/repository/opamVCS.mli | 6 +- tests/reftests/dune.inc | 17 ++ tests/reftests/working-dir.test | 284 ++++++++++++++++++++++++++++++++ 8 files changed, 329 insertions(+), 7 deletions(-) create mode 100644 tests/reftests/working-dir.test diff --git a/CHANGES b/CHANGES index bddd5f5c637..deeaa0af287 100644 --- a/CHANGES +++ b/CHANGES @@ -29,6 +29,9 @@ are not marked). Those prefixed with "(+)" are new command/option (since * [BUG] Handle external dependencies when updating switch state pin status (all pins), instead as a post pin action (only when called with `opam pin` [#5047 @rjbou - fix #5046] +* [BUG] When reinstalling a package that has a dirty source, if uncommitted + changes are the same than the ones stored in opam's cache, opam consider that + it is up to date and nothing is updated [4879 @rjbou] * Stop Zypper from upgrading packages on updates on OpenSUSE [#4978 @kit-ty-kate] * Clearer error message if a command doesn't exist diff --git a/src/repository/opamDarcs.ml b/src/repository/opamDarcs.ml index 9f37b32a340..95b75c4f282 100644 --- a/src/repository/opamDarcs.ml +++ b/src/repository/opamDarcs.ml @@ -81,10 +81,14 @@ module VCS = struct OpamSystem.raise_on_process_error r; Done () - let reset_tree repo_root _repo_url = + let clean repo_root = darcs repo_root [ "obliterate"; "--all"; "-t"; opam_local_tag ] @@> fun r -> OpamSystem.raise_on_process_error r; + Done () + + let reset_tree repo_root _repo_url = + clean repo_root @@+ fun () -> darcs repo_root [ "obliterate"; "--all"; "-p"; opam_reverse_commit ] @@> fun r -> (* returns 0 even if patch doesn't exist *) diff --git a/src/repository/opamGit.ml b/src/repository/opamGit.ml index 73e9a13ab55..9af216c9f71 100644 --- a/src/repository/opamGit.ml +++ b/src/repository/opamGit.ml @@ -162,6 +162,12 @@ module VCS : OpamVCS.VCS = struct else Done (Some full)) + let clean repo_root = + git repo_root [ "clean"; "-fdx" ] + @@> fun r -> + OpamSystem.raise_on_process_error r; + Done () + let reset_tree repo_root repo_url = let rref = remote_ref repo_url in git repo_root [ "reset" ; "--hard"; rref; "--" ] @@ -169,11 +175,7 @@ module VCS : OpamVCS.VCS = struct if OpamProcess.is_failure r then OpamSystem.internal_error "Git error: %s not found." rref else - git repo_root [ "clean"; "-fdx" ] - @@> fun r -> - if OpamProcess.is_failure r then - OpamSystem.internal_error "Git error: %s not found." rref - else + clean repo_root @@+ fun () -> if OpamFilename.exists (repo_root // ".gitmodules") then git repo_root [ "submodule"; "update"; "--init"; "--recursive" ] @@> fun r -> diff --git a/src/repository/opamHg.ml b/src/repository/opamHg.ml index 1e29e404339..bb40719b64d 100644 --- a/src/repository/opamHg.ml +++ b/src/repository/opamHg.ml @@ -56,6 +56,12 @@ module VCS = struct if String.length full > 8 then Done (Some (String.sub full 0 8)) else Done (Some full) + let clean repo_root = + hg repo_root ["revert"; "--all"; "--no-backup"] + @@> fun r -> + OpamSystem.raise_on_process_error r; + Done () + let reset_tree repo_root repo_url = let mark = mark_from_url repo_url in hg repo_root [ "update"; "--clean"; "--rev"; mark ] @@> fun r -> diff --git a/src/repository/opamVCS.ml b/src/repository/opamVCS.ml index 6a3902d4edb..3fd822ede6a 100644 --- a/src/repository/opamVCS.ml +++ b/src/repository/opamVCS.ml @@ -29,6 +29,7 @@ module type VCS = sig val is_dirty: ?subpath:string -> dirname -> bool OpamProcess.job val modified_files: dirname -> string list OpamProcess.job val get_remote_url: ?hash:string -> dirname -> url option OpamProcess.job + val clean: dirname -> unit OpamProcess.job end let convert_path = @@ -83,6 +84,7 @@ module Make (VCS: VCS) = struct Done (Not_available (None, OpamUrl.to_string url))) @@ fun () -> if VCS.exists dirname then + VCS.clean dirname @@+ fun () -> VCS.fetch ?cache_dir ?subpath dirname url @@+ fun () -> VCS.is_up_to_date dirname url @@+ function | true -> Done (Up_to_date None) diff --git a/src/repository/opamVCS.mli b/src/repository/opamVCS.mli index af051e32c9f..ff0ec947d91 100644 --- a/src/repository/opamVCS.mli +++ b/src/repository/opamVCS.mli @@ -68,10 +68,14 @@ module type VCS = sig val is_dirty: ?subpath:string -> dirname -> bool OpamProcess.job (** Returns the list of files under version control, modified in the working - tree but not comitted *) + tree but not committed *) val modified_files: dirname -> string list OpamProcess.job + (* Returns associated remote url, if found *) val get_remote_url: ?hash:string -> dirname -> url option OpamProcess.job + + (* Remove uncommitted changes *) + val clean: dirname -> unit OpamProcess.job end (** Create a backend from a [VCS] implementation. *) diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index aea5c99c240..ff44051d6d4 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -532,6 +532,23 @@ %{targets} (run ./run.exe %{bin:opam} %{dep:var-option.test} %{read-lines:testing-env})))) +(alias + (name reftest-working-dir) + (action + (diff working-dir.test working-dir.out))) + +(alias + (name reftest) + (deps (alias reftest-working-dir))) + +(rule + (targets working-dir.out) + (deps root-N0REP0) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{bin:opam} %{dep:working-dir.test} %{read-lines:testing-env})))) + (rule (targets opam-repo-N0REP0) (action diff --git a/tests/reftests/working-dir.test b/tests/reftests/working-dir.test new file mode 100644 index 00000000000..93f574a8654 --- /dev/null +++ b/tests/reftests/working-dir.test @@ -0,0 +1,284 @@ +N0REP0 +### OPAMYES=1 OPAMJOBS=1 +### +opam-version: "2.0" +build: [[ "test" "-f" "ongoing.txt" ] [ "cat" "ongoing.txt" ]] +### +versionned +### git -C ./ongoing init -q +### git -C ./ongoing config core.autocrlf false +### git -C ./ongoing add -A +### git -C ./ongoing commit -qm "init" +### opam switch create working-dir --empty +### opam pin ./ongoing +Package ongoing does not exist, create as a NEW package? [Y/n] y +ongoing is now pinned to git+file://${BASEDIR}/ongoing#master (version ~dev) + +The following actions will be performed: + - install ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> retrieved ongoing.~dev (no changes) +-> installed ongoing.~dev +Done. +### +new! +### +opam-version: "2.0" +build: [[ "test" "-f" "newfile.txt" ] [ "cat" "newfile.txt" ]] +### opam remove ongoing +The following actions will be performed: + - remove ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed ongoing.~dev +Done. +### opam install ongoing -v | '.*(/|\\|")test(\.exe)?"? "' -> 'test "' | '.*(/|\\|")cat(\.exe)?"? "' -> 'cat "' + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: +[NOTE] Ignoring uncommitted changes in ${BASEDIR}/ongoing (`--working-dir' not active). +Processing 1/1: [ongoing.~dev: git] +[ongoing.~dev] synchronised (no changes) + +The following actions will be performed: + - install ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/3: [ongoing.~dev: git] +-> retrieved ongoing.~dev (no changes) +Processing 2/3: [ongoing: test ongoing.txt] +test "-f" "ongoing.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +Processing 2/3: [ongoing: cat ongoing.txt] +cat "ongoing.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +- versionned +-> compiled ongoing.~dev +-> installed ongoing.~dev +Done. +### opam install ongoing --working-dir -v | '.*(/|\\|")test(\.exe)?"? "' -> 'test "' | '.*(/|\\|")cat(\.exe)?"? "' -> 'cat "' + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [ongoing.~dev: git] +[ongoing.~dev] synchronised (git+file://${BASEDIR}/ongoing#master) +[ongoing] Installing new package description from upstream git+file://${BASEDIR}/ongoing#master + +The following actions will be performed: + - recompile ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/3: [ongoing: test newfile.txt] +test "-f" "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +Processing 1/3: [ongoing: cat newfile.txt] +cat "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +- new! +-> compiled ongoing.~dev +-> removed ongoing.~dev +-> installed ongoing.~dev +Done. +### git -C ./ongoing add ongoing.opam +### git -C ./ongoing commit -qm "forgot a file" +### opam remove ongoing +The following actions will be performed: + - remove ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed ongoing.~dev +Done. +### opam install ongoing -v | grep -v "^#" | '.*(/|\\|")test(\.exe)?"? "' -> 'test "' + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: +[NOTE] Ignoring uncommitted changes in ${BASEDIR}/ongoing (`--working-dir' not active). +Processing 1/1: [ongoing.~dev: git] +[ongoing.~dev] synchronised (no changes) + +The following actions will be performed: + - install ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/3: [ongoing.~dev: git] +-> retrieved ongoing.~dev (no changes) +Processing 2/3: [ongoing: test newfile.txt] +test "-f" "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +[ERROR] The compilation of ongoing.~dev failed at "test -f newfile.txt". + + + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - build ongoing ~dev ++- +- No changes have been performed +'${OPAM} install ongoing -v' failed. +# Return code 31 # +### opam install ongoing --working-dir -v | '.*(/|\\|")test(\.exe)?"? "' -> 'test "' | '.*(/|\\|")cat(\.exe)?"? "' -> 'cat "' + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [ongoing.~dev: git] +[ongoing.~dev] synchronised (git+file://${BASEDIR}/ongoing#master) + +The following actions will be performed: + - install ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/2: [ongoing: test newfile.txt] +test "-f" "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +Processing 1/2: [ongoing: cat newfile.txt] +cat "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +- new! +-> compiled ongoing.~dev +-> installed ongoing.~dev +Done. +### : working dir and not pinned packages +### +opam-version:"2.0" +build:[ "test" "-f" "present"] +### +basedir=`echo $BASEDIR | sed "s/\\\\\\\\/\\\\\\\\\\\\\\\\/g"` +echo "url { src:\"file://${basedir}/qux\" }" >> REPO/packages/qux/qux.4/opam +### sh mkurl.sh +### opam update default + +<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><> +[default] synchronised from file://${BASEDIR}/REPO +Now run 'opam upgrade' to apply any package updates. +### +true +### +opam-version: "2.0" +build: [[ "test" "-f" "ongoing.txt" ] [ "cat" "ongoing.txt" ]] +### git -C ./ongoing add ongoing.opam +### git -C ./ongoing commit -qm "mixed wd" +### +opam-version: "2.0" +build: [[ "test" "-f" "newfile.txt" ] [ "cat" "newfile.txt" ]] +### opam install qux +The following actions will be performed: + - install qux 4 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> retrieved qux.4 (file://${BASEDIR}/qux) +-> installed qux.4 +Done. +### opam remove qux +The following actions will be performed: + - remove qux 4 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed qux.4 +Done. +### opam install qux --working-dir +The following actions will be performed: + - install qux 4 + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +[ERROR] The compilation of qux.4 failed at "test -f present". + + + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - build qux 4 ++- +- No changes have been performed +# Return code 31 # +### opam remove qux ongoing +[NOTE] qux is not installed. + +The following actions will be performed: + - remove ongoing ~dev* + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed ongoing.~dev +Done. +### +opam-version: "2.0" +### git -C ./ongoing add ongoing.opam +### git -C ./ongoing commit -qm "mixed wd" +### +opam-version: "2.0" +build: [[ "test" "-f" "newfile.txt" ] [ "cat" "newfile.txt" ]] +### opam install qux ongoing --working-dir -v | '.*(/|\\|")test(\.exe)?"? "' -> 'test "' | '.*(/|\\|")cat(\.exe)?"? "' -> 'cat "' | grep -v "switch import" + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/1: [ongoing.~dev: git] +[ongoing.~dev] synchronised (git+file://${BASEDIR}/ongoing#master) + +The following actions will be performed: + - install qux 4 + - install ongoing ~dev* +===== 2 to install ===== + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +Processing 1/4: [ongoing: test newfile.txt] +test "-f" "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +Processing 1/4: [ongoing: cat newfile.txt] +cat "newfile.txt" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/ongoing.~dev) +- new! +-> compiled ongoing.~dev +Processing 2/4: [qux: test present] +-> installed ongoing.~dev +Processing 3/4: [qux: test present] +test "-f" "present" (CWD=${BASEDIR}/OPAM/working-dir/.opam-switch/build/qux.4) +[ERROR] The compilation of qux.4 failed at "test -f present". + + + + +<><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><> ++- The following actions failed +| - build qux 4 ++- ++- The following changes have been performed +| - install ongoing ~dev ++- + +The former state can be restored with: +'${OPAM} install qux ongoing --working-dir -v' failed. +# Return code 31 # +### : Shows that opam correctly handles updated constraints with --working-dir +### : See https://github.com/ocaml/opam/issues/5178 +### opam pin remove ./ongoing +Ok, ongoing is no longer pinned to git+file://${BASEDIR}/ongoing#master (version ~dev) +The following actions will be performed: + - remove ongoing ~dev + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> removed ongoing.~dev +Done. +### +opam-version:"2.0" +### +opam-version: "2.0" +depends: ["qux"] +### opam install ./ongoing --working-dir +Package ongoing does not exist, create as a NEW package? [Y/n] y +ongoing is now pinned to git+file://${BASEDIR}/ongoing#master (version ~dev) +[ongoing.~dev] synchronised (git+file://${BASEDIR}/ongoing#master) +The following actions will be performed: + - install qux 4 [required by ongoing] + - install ongoing ~dev* +===== 2 to install ===== + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> retrieved qux.4 (file://${BASEDIR}/qux) +-> installed qux.4 +-> installed ongoing.~dev +Done. +### +opam-version: "2.0" +depends: ["qux" {= "1"}] +### opam install ./ongoing --working-dir --show-action +[NOTE] Package ongoing is already installed (current version is ~dev). +### opam reinstall ./ongoing --working-dir --show-action + +<><> Synchronising pinned packages ><><><><><><><><><><><><><><><><><><><><><><> +[ongoing.~dev] synchronised (git+file://${BASEDIR}/ongoing#master) +[ongoing] Installing new package description from upstream git+file://${BASEDIR}/ongoing#master + +The following actions would be performed: + - downgrade qux 4 to 1 [required by ongoing] + - recompile ongoing ~dev* +===== 1 to recompile | 1 to downgrade =====