Fail when trying to pin a package whose definition could not be found instead of forcing interactive edition#6319
Conversation
| OpamConsole.note | ||
| "No package definition found for %s: please complete the template" | ||
| (OpamConsole.colorise `bold (OpamPackage.to_string nv)); | ||
| OpamFileTools.template nv |
There was a problem hiding this comment.
do we really want to keep this template around? In which situation is it useful? I feel like it doesn't really has any purpose and we might want to remove it entirely (probably in a separate PR)
There was a problem hiding this comment.
In the case of an opam pin edit and there is no opam file (we want to create one)
607c6cd to
39b1927
Compare
39b1927 to
5e0a66d
Compare
|
Discussion on dev meeting: Overall agreement on blocked CI issue. We can't set the editing only on non tty as it is used hackishly in several places (our test for ex.) with specific command in OPAMEDITOR (cp/sed/etc.). |
6a1afb5 to
ecf85c4
Compare
ecf85c4 to
5f3ce67
Compare
5f3ce67 to
95a905d
Compare
rjbou
left a comment
There was a problem hiding this comment.
On the general implem, LGTM! some small remarks
| OpamConsole.note | ||
| "No package definition found for %s: please complete the template" | ||
| (OpamConsole.colorise `bold (OpamPackage.to_string nv)); | ||
| OpamFileTools.template nv |
There was a problem hiding this comment.
In the case of an opam pin edit and there is no opam file (we want to create one)
| OpamFile.OPAM.(with_metadata_dir (metadata_dir opam_base)) | ||
| else | ||
| Some opam_base | ||
| Option.map (OpamFile.OPAM.with_url_opt urlf) opam_opt |
There was a problem hiding this comment.
As we update url here, do we still need lines 600-604 ?
let opam =
match OpamFile.OPAM.get_url opam with
| Some _ -> opam
| None -> OpamFile.OPAM.with_url_opt urlf opam
inThere was a problem hiding this comment.
good catch. That was also the case in the code before this PR but it's always good to clean redundant code ^^
There was a problem hiding this comment.
done in follow up fixup commit
There was a problem hiding this comment.
actually looking at it more after the reftests failed, this wasn't redundant after all.
When editing users can remove the url part so a check after always needs to happen, and when not editing we want to force the url to be urlf.
I removed that fixup commit.
67d52ef to
7cc951b
Compare
…g opam description
… instead of forcing interactive edition
7cc951b to
4b42c86
Compare
Closes #6322
This could happen with things like
opam pin <typo> <url>or a typo in thepin-dependsdefinition.The interactive edition based on the dummy opam file template is now only allowed if the user is purposefully requesting it using
--editoropam pin edit.