From ed4192a846162f0a6936b799f05a64e863d6b785 Mon Sep 17 00:00:00 2001 From: Kate Date: Sat, 30 Nov 2024 16:58:28 +0000 Subject: [PATCH 1/3] Add tests showing behaviour of opam pin when confronted with a missing opam description --- tests/reftests/pin.test | 105 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/tests/reftests/pin.test b/tests/reftests/pin.test index 0f70077adb5..cbe997a4d97 100644 --- a/tests/reftests/pin.test +++ b/tests/reftests/pin.test @@ -937,3 +937,108 @@ vcs-local is now pinned to git+file://${BASEDIR}/vcs-local#master (version dev) ### opam show vcs-local --field build:,url.src: build: "false" url.src: "git+file://${BASEDIR}/vcs-local#master" +### : behaviour of opam pin when confronted with a missing opam description +### OPAMEDITOR="echo iNvAlId " +### +#!/bin/sh +set -eu +basedir=$(echo "$BASEDIR" | sed "s/\\\\\\\\/\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\/g") +cat << EOF >> "$1" +pin-depends: [ + ["$2" "file://$basedir/$3"] +] +EOF +### +opam-version: "2.0" +### opam pin pkg-with-typo ./missing-opam --no-action +Package pkg-with-typo does not exist, create as a NEW package? [Y/n] y +[pkg-with-typo.dev] synchronised (file://${BASEDIR}/missing-opam) +[NOTE] No package definition found for pkg-with-typo.dev: please complete the template +iNvAlId ${BASEDIR}/OPAM/vcs-local/.opam-switch/overlay/pkg-with-typo/opam_ +[WARNING] The opam file didn't pass validation: + error 22: Some fields are present but empty; remove or fill them: "homepage", "license", "bug_reports" + error 57: Synopsis must not be empty + warning 62: License doesn't adhere to the SPDX standard, see https://spdx.org/licenses/ : "" +Proceed anyway ('no' will re-edit)? [Y/n] y +You can edit this file again with "opam pin edit pkg-with-typo", export it with "opam show pkg-with-typo --raw" +pkg-with-typo is now pinned to file://${BASEDIR}/missing-opam (version dev) +### opam pin pkg-with-typo ./missing-opam +[NOTE] Package pkg-with-typo is already pinned to file://${BASEDIR}/missing-opam (version dev). +[pkg-with-typo.dev] synchronised (no changes) +pkg-with-typo is now pinned to file://${BASEDIR}/missing-opam (version dev) + +[ERROR] Package conflict! + * Missing dependency: + - pkg-with-typo -> specify-dependencies-here + unknown package + +[NOTE] Pinning command successful, but your installed packages may be out of sync. +# Return code 20 # +### +opam-version: "2.0" +### sh ./pin-depends.sh missing-opam/pkg.opam pin-pkg.dev missing-opam +### opam pin ./missing-opam +Package pkg does not exist, create as a NEW package? [Y/n] y +The following additional pinnings are required by pkg.dev: + - pin-pkg.dev at file://${BASEDIR}/missing-opam +Pin and install them? [Y/n] y +Package pin-pkg does not exist, create as a NEW package? [Y/n] y +[pin-pkg.dev] synchronised (no changes) +[NOTE] No package definition found for pin-pkg.dev: please complete the template +iNvAlId ${BASEDIR}/OPAM/vcs-local/.opam-switch/overlay/pin-pkg/opam_ +[WARNING] The opam file didn't pass validation: + error 22: Some fields are present but empty; remove or fill them: "homepage", "license", "bug_reports" + error 57: Synopsis must not be empty + warning 62: License doesn't adhere to the SPDX standard, see https://spdx.org/licenses/ : "" +Proceed anyway ('no' will re-edit)? [Y/n] y +You can edit this file again with "opam pin edit pin-pkg", export it with "opam show pin-pkg --raw" +pin-pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) +pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) + +[ERROR] Package conflict! + * Missing dependency: + - pin-pkg -> specify-dependencies-here + unknown package + +[NOTE] Pinning command successful, but your installed packages may be out of sync. +# Return code 20 # +### +opam-version: "2.0" +depends: [ + "pin-pkg" +] +### sh ./pin-depends.sh missing-opam/pkg.opam pin-pkg.dev missing-opam +### opam pin ./missing-opam +[NOTE] Package pkg is already pinned to file://${BASEDIR}/missing-opam (version dev). +pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) + +[ERROR] Package conflict! + * Missing dependency: + - pkg -> pin-pkg -> specify-dependencies-here + unknown package + +[NOTE] Pinning command successful, but your installed packages may be out of sync. +# Return code 20 # +### : special case where the name of the pin-depend matches the name of the package itself (recursive) +### +opam-version: "2.0" +### sh ./pin-depends.sh missing-opam/pkg.opam pkg.dev missing-opam/empty-dir +### mkdir missing-opam/empty-dir +### opam pin ./missing-opam +[NOTE] Package pkg is already pinned to file://${BASEDIR}/missing-opam (version dev). +The following additional pinnings are required by pkg.dev: + - pkg.dev at file://${BASEDIR}/missing-opam/empty-dir +Pin and install them? [Y/n] y +[NOTE] Package pkg is currently pinned to file://${BASEDIR}/missing-opam (version dev). +[pkg.dev] synchronised (no changes) +pkg is now pinned to file://${BASEDIR}/missing-opam/empty-dir (version dev) +pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) + +The following actions will be performed: +=== install 1 package + - install pkg dev (pinned) + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> retrieved pkg.dev (file://${BASEDIR}/missing-opam) +-> installed pkg.dev +Done. From 5e0a66d8d45a34a12c116c1062f1feb855fe21df Mon Sep 17 00:00:00 2001 From: Kate Date: Sat, 30 Nov 2024 16:06:21 +0000 Subject: [PATCH 2/3] Fail when trying to pin a package whose definition could not be found instead of forcing interactive edition --- master_changes.md | 1 + src/client/opamPinCommand.ml | 42 ++++++++--------- tests/reftests/pin.test | 91 +++++++++--------------------------- 3 files changed, 41 insertions(+), 93 deletions(-) diff --git a/master_changes.md b/master_changes.md index 64d2b77cc2e..0b78b4c17c5 100644 --- a/master_changes.md +++ b/master_changes.md @@ -44,6 +44,7 @@ users) ## Pin * [NEW] Make it so pin list display the current revision of a pinned repository in a new column [#6274 @desumn - fix #5533] + * [BUG] Fail when trying to pin a package whose definition could not be found instead of forcing interactive edition (e.g. this could happen when making a typo in the package name of a pin-depends) [#6319 @kit-ty-kate - fix ocaml/setup-ocaml#902] ## List diff --git a/src/client/opamPinCommand.ml b/src/client/opamPinCommand.ml index 3a4506fb538..af4645d8228 100644 --- a/src/client/opamPinCommand.ml +++ b/src/client/opamPinCommand.ml @@ -571,32 +571,28 @@ and source_pin | _ -> opam_opt in - if not need_edit && opam_opt = None then - OpamConsole.note - "No package definition found for %s: please complete the template" - (OpamConsole.colorise `bold (OpamPackage.to_string nv)); - - let need_edit = need_edit || opam_opt = None in - let opam_opt = - let opam_base = match opam_opt with - | None -> OpamFileTools.template nv - | Some opam -> opam - in - let opam_base = - OpamFile.OPAM.with_url_opt urlf opam_base - in if need_edit then - (if not (OpamFile.exists temp_file) then - OpamFile.OPAM.write_with_preserved_format - ?format_from:(OpamPinned.orig_opam_file st name opam_base) - temp_file opam_base; - edit_raw name temp_file >>| - (* Preserve metadata_dir so that copy_files below works *) - OpamFile.OPAM.(with_metadata_dir (metadata_dir opam_base)) - ) + let opam_base = match opam_opt with + | None -> + OpamConsole.note + "No package definition found for %s: please complete the template" + (OpamConsole.colorise `bold (OpamPackage.to_string nv)); + OpamFileTools.template nv + | Some opam -> opam + in + let opam_base = + OpamFile.OPAM.with_url_opt urlf opam_base + in + if not (OpamFile.exists temp_file) then + OpamFile.OPAM.write_with_preserved_format + ?format_from:(OpamPinned.orig_opam_file st name opam_base) + temp_file opam_base; + edit_raw name temp_file >>| + (* Preserve metadata_dir so that copy_files below works *) + OpamFile.OPAM.(with_metadata_dir (metadata_dir opam_base)) else - Some opam_base + Option.map (OpamFile.OPAM.with_url_opt urlf) opam_opt in match opam_opt with | None -> diff --git a/tests/reftests/pin.test b/tests/reftests/pin.test index cbe997a4d97..13b6385308a 100644 --- a/tests/reftests/pin.test +++ b/tests/reftests/pin.test @@ -682,17 +682,9 @@ Done. nip-v is now pinned to file://${BASEDIR}/nip-v.1.tgz (version 2) ### opam pin add nip 1 --with-version 42 --no-action [nip.42] synchronised (file://${BASEDIR}/nip.1.tgz) -[NOTE] No package definition found for nip.42: please complete the template -[WARNING] The opam file didn't pass validation: - error 22: Some fields are present but empty; remove or fill them: "homepage", "license", "bug_reports" - error 57: Synopsis must not be empty - warning 59: url doesn't contain a checksum - warning 62: License doesn't adhere to the SPDX standard, see https://spdx.org/licenses/ : "" -Proceed anyway ('no' will re-edit)? [Y/n] y -You can edit this file again with "opam pin edit nip", export it with "opam show nip --raw" -nip is now pinned to file://${BASEDIR}/nip.1.tgz (version 42) +[ERROR] No valid package definition found +# Return code 5 # ### opam pin list -nip.42 (uninstalled) rsync file://${BASEDIR}/nip.1.tgz nip-path.42 rsync file://${BASEDIR}/nip-path nip-v.2 (uninstalled) rsync file://${BASEDIR}/nip-v.1.tgz ### find OPAM/versions/lib -name '*.t' | sort @@ -953,27 +945,13 @@ opam-version: "2.0" ### opam pin pkg-with-typo ./missing-opam --no-action Package pkg-with-typo does not exist, create as a NEW package? [Y/n] y [pkg-with-typo.dev] synchronised (file://${BASEDIR}/missing-opam) -[NOTE] No package definition found for pkg-with-typo.dev: please complete the template -iNvAlId ${BASEDIR}/OPAM/vcs-local/.opam-switch/overlay/pkg-with-typo/opam_ -[WARNING] The opam file didn't pass validation: - error 22: Some fields are present but empty; remove or fill them: "homepage", "license", "bug_reports" - error 57: Synopsis must not be empty - warning 62: License doesn't adhere to the SPDX standard, see https://spdx.org/licenses/ : "" -Proceed anyway ('no' will re-edit)? [Y/n] y -You can edit this file again with "opam pin edit pkg-with-typo", export it with "opam show pkg-with-typo --raw" -pkg-with-typo is now pinned to file://${BASEDIR}/missing-opam (version dev) +[ERROR] No valid package definition found +# Return code 5 # ### opam pin pkg-with-typo ./missing-opam -[NOTE] Package pkg-with-typo is already pinned to file://${BASEDIR}/missing-opam (version dev). +Package pkg-with-typo does not exist, create as a NEW package? [Y/n] y [pkg-with-typo.dev] synchronised (no changes) -pkg-with-typo is now pinned to file://${BASEDIR}/missing-opam (version dev) - -[ERROR] Package conflict! - * Missing dependency: - - pkg-with-typo -> specify-dependencies-here - unknown package - -[NOTE] Pinning command successful, but your installed packages may be out of sync. -# Return code 20 # +[ERROR] No valid package definition found +# Return code 5 # ### opam-version: "2.0" ### sh ./pin-depends.sh missing-opam/pkg.opam pin-pkg.dev missing-opam @@ -984,24 +962,8 @@ The following additional pinnings are required by pkg.dev: Pin and install them? [Y/n] y Package pin-pkg does not exist, create as a NEW package? [Y/n] y [pin-pkg.dev] synchronised (no changes) -[NOTE] No package definition found for pin-pkg.dev: please complete the template -iNvAlId ${BASEDIR}/OPAM/vcs-local/.opam-switch/overlay/pin-pkg/opam_ -[WARNING] The opam file didn't pass validation: - error 22: Some fields are present but empty; remove or fill them: "homepage", "license", "bug_reports" - error 57: Synopsis must not be empty - warning 62: License doesn't adhere to the SPDX standard, see https://spdx.org/licenses/ : "" -Proceed anyway ('no' will re-edit)? [Y/n] y -You can edit this file again with "opam pin edit pin-pkg", export it with "opam show pin-pkg --raw" -pin-pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) -pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) - -[ERROR] Package conflict! - * Missing dependency: - - pin-pkg -> specify-dependencies-here - unknown package - -[NOTE] Pinning command successful, but your installed packages may be out of sync. -# Return code 20 # +[ERROR] No valid package definition found +# Return code 5 # ### opam-version: "2.0" depends: [ @@ -1009,36 +971,25 @@ depends: [ ] ### sh ./pin-depends.sh missing-opam/pkg.opam pin-pkg.dev missing-opam ### opam pin ./missing-opam -[NOTE] Package pkg is already pinned to file://${BASEDIR}/missing-opam (version dev). -pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) - -[ERROR] Package conflict! - * Missing dependency: - - pkg -> pin-pkg -> specify-dependencies-here - unknown package - -[NOTE] Pinning command successful, but your installed packages may be out of sync. -# Return code 20 # +Package pkg does not exist, create as a NEW package? [Y/n] y +The following additional pinnings are required by pkg.dev: + - pin-pkg.dev at file://${BASEDIR}/missing-opam +Pin and install them? [Y/n] y +Package pin-pkg does not exist, create as a NEW package? [Y/n] y +[pin-pkg.dev] synchronised (no changes) +[ERROR] No valid package definition found +# Return code 5 # ### : special case where the name of the pin-depend matches the name of the package itself (recursive) ### opam-version: "2.0" ### sh ./pin-depends.sh missing-opam/pkg.opam pkg.dev missing-opam/empty-dir ### mkdir missing-opam/empty-dir ### opam pin ./missing-opam -[NOTE] Package pkg is already pinned to file://${BASEDIR}/missing-opam (version dev). +Package pkg does not exist, create as a NEW package? [Y/n] y The following additional pinnings are required by pkg.dev: - pkg.dev at file://${BASEDIR}/missing-opam/empty-dir Pin and install them? [Y/n] y -[NOTE] Package pkg is currently pinned to file://${BASEDIR}/missing-opam (version dev). +Package pkg does not exist, create as a NEW package? [Y/n] y [pkg.dev] synchronised (no changes) -pkg is now pinned to file://${BASEDIR}/missing-opam/empty-dir (version dev) -pkg is now pinned to file://${BASEDIR}/missing-opam (version dev) - -The following actions will be performed: -=== install 1 package - - install pkg dev (pinned) - -<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> --> retrieved pkg.dev (file://${BASEDIR}/missing-opam) --> installed pkg.dev -Done. +[ERROR] No valid package definition found +# Return code 5 # From 47a607e30c9fc4dad9f852ad4da6befa84f804ec Mon Sep 17 00:00:00 2001 From: Kate Date: Sun, 1 Dec 2024 16:46:04 +0000 Subject: [PATCH 3/3] fixup --- tests/reftests/pin.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/reftests/pin.test b/tests/reftests/pin.test index 13b6385308a..80e5cc2bdde 100644 --- a/tests/reftests/pin.test +++ b/tests/reftests/pin.test @@ -934,7 +934,7 @@ url.src: "git+file://${BASEDIR}/vcs-local#master" ### #!/bin/sh set -eu -basedir=$(echo "$BASEDIR" | sed "s/\\\\\\\\/\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\\/g") +basedir=$(echo "$BASEDIR" | sed 's/\\/\\\\\\\\/g') cat << EOF >> "$1" pin-depends: [ ["$2" "file://$basedir/$3"]