On install action, reinstall if test/doc dependencies are not present#4514
On install action, reinstall if test/doc dependencies are not present#4514rjbou wants to merge 2 commits intoocaml:masterfrom
Conversation
|
Current state : it filters according |
|
Dev meeting: Pushing this out to 2.1.1, we can make sure the right variables are all accounted for. There is a workaround for |
AltGr
left a comment
There was a problem hiding this comment.
Hm, this should make the issue less pregnant, but isn't a complete solution: indeed, if you opam install foo --with-doc and doc-deps were missing, it will install them, and then recompile foo (you don't even need to add to reinstall in this case, the dependency change will trigger it already ; just removing from pkg_skip would have been enough) : you get the doc for foo installed as expected. But if the doc dependencies happened to be installed already, you will still get a nothing to do without a recompilation with docs enabled.
We don't really have a way to fix this since we don't record (atm) the flags that were set when foo was initially installed, so the only options would be to always reinstall when --with-x is set (that sounds dangerous ; but it does actually make complete sense in the case of tests ? Maybe after a question like for opam reinstall <not-installed-pkg> ?) ; or just advise to use reinstall instead ?
| in | ||
| let pkg_skip, pkg_reinstall = | ||
| if OpamStateConfig.(!r.build_test || !r.build_doc) then | ||
| let to_reinstall = |
There was a problem hiding this comment.
Wouldn't it be a bit simpler here to just resolve the dependencies in the current context (i.e. following the OpamStateConfig.r.build_* values), and check that everything is installed ?
1653ffc to
8ac38f9
Compare
|
I think this should be closed in favour of #6209 which fixes the root cause |
|
Issues are not the same, this one is not specific to pinned packages, but more generally to packages with predefined variables that can be specified via cli. |
fix #4507